Skip to content

HHH-19551 - Address deficiencies in pessimistic locking #10194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sebersole
Copy link
Member

@sebersole sebersole commented May 21, 2025

HHH-19551 - Address deficiencies in pessimistic locking

[Please describe here what your change is about]


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19551

@gavinking
Copy link
Member

Excellent!

@sebersole
Copy link
Member Author

Unless I did something silly, this should be just Oracle failures now. Need to investigate those more.

@sebersole sebersole force-pushed the lock-scope-2 branch 2 times, most recently from eaede26 to 321fd2e Compare June 4, 2025 21:47
@sebersole sebersole force-pushed the lock-scope-2 branch 2 times, most recently from 776d5f6 to e0e279e Compare June 16, 2025 19:29
@sebersole sebersole force-pushed the lock-scope-2 branch 2 times, most recently from 535fa89 to 00f0c68 Compare June 20, 2025 15:40
@@ -69,6 +71,10 @@
registerKeywords( info );
}

protected LockingSupport buildLockingSupport() {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
MySQLLegacyDialect.buildLockingSupport
; it is advisable to add an Override annotation.
private final LockTimeoutType noWaitType;
private final LockTimeoutType waitType;

public MariaDBLockingSupport(boolean supportsSkipLocked, boolean supportsNoWait, boolean supportsWait) {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'supportsWait' is never used.
@sebersole sebersole changed the title HHH-19336 - Proper implementation for JPA extended locking scope HHH-19551 - Address deficiencies in pessimistic locking Jun 20, 2025
@sebersole
Copy link
Member Author

About to head out for ~2 weeks of PTO...

About test failures:

  1. MariaDB - one test fails (LockedRowsTests#testFindSkipLocked). It does not fail locally if I just run that test or run all the tests in that class or run all the tests in that package or ... makes it hard to debug, but I'll gwt back to it after PTO
  2. Oracle - I have not even started digging into these. I'm 100% certain it is because of the way AbstractSqlAstTranslator has LOTS of code specific to Oracle. Handling of for-update was definitely part of that.

Some things I'd still like accomplish here...

  1. The main one is to move rendering of the lock fragments from Dialect proper into the LockingSupport impls.
  2. A related task is to make LockingSupport.Metadata sensitive to write/read locking. I think tjis will be needed for (1).
  3. Integrate support for LockTimeoutType#CONNECTION into the acquisition of locks when performing find/lock/refresh operations. I've implemented ConnectionLockTimeoutStrategy for quite a few dialects already, which handles the actual setting/resetting.
  4. Adjust design of JdbcOperation to simply be a concept of something that needs to be performed via JDBC. Currently it implies a single statement. Ultimately I want this to be open-ended and mean 1-or-more statements. This would allow grouping all the SQL needed for locking into a single JdbcOperation and combine ConnectionLockTimeoutStrategy, follow-on locking as well as the main query.

WRT (2), currently I have:

public interface LockingSupport {
	Metadata getMetadata();

	...

	interface Metadata {
		LockTimeoutType getLockTimeoutType(Timeout timeout);
		RowLockStrategy getReadRowLockStrategy();
		RowLockStrategy getWriteRowLockStrategy();
		...
	}
}

public class Dialect {
	...
	LockingSupport getLockingSupport();
}

Thinking instead of something like:

public interface LockingSupport {
	Metadata getMetadata();

	...

	interface Metadata {
		LockTimeoutType getLockTimeoutType(Timeout timeout);
		RowLockStrategy getRowLockStrategy();
		...
	}
}

public class Dialect {
	...
	LockingSupport getWriteLockingSupport();
	LockingSupport getReadLockingSupport();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants