Skip to content

Commit bab21da

Browse files
beikovbeikov
committed
Apply session options to enable proper locking for CockroachDB
This fixes issues with ScopeTests when running against CockroachDB. Also remove test skips that were due to locking not working correctly. See cockroachdb/cockroach#88995 for details
1 parent baa9aa5 commit bab21da

File tree

7 files changed

+9
-16
lines changed

7 files changed

+9
-16
lines changed

‎hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/CockroachLockingSupport.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ public LockTimeoutType getLockTimeoutType(Timeout timeout) {
5555
return switch (timeout.milliseconds()) {
5656
case WAIT_FOREVER_MILLI -> QUERY;
5757
case NO_WAIT_MILLI -> supportsNoWait ? QUERY : LockTimeoutType.NONE;
58+
// Due to https://github.com/cockroachdb/cockroach/issues/88995, locking doesn't work properly
59+
// without certain configuration options and hence skipping locked rows also doesn't work correctly.
5860
case SKIP_LOCKED_MILLI -> LockTimeoutType.NONE;
5961
// it does not, however, support WAIT as part of for-update, but does support a connection-level lock_timeout setting
6062
default -> CONNECTION;

‎hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockExceptionTests.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ protected void tearDown() {
5353

5454
@Test
5555
@JiraKey( value = "HHH-8786" )
56-
@SkipForDialect(dialectClass = CockroachDialect.class, reason = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
5756
@SkipForDialect(dialectClass = InformixDialect.class, reason = "no failure")
5857
public void testLockTimeoutFind() {
5958
final Item item = new Item( "find" );

‎hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockTest.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ public void testFindWithTimeoutHint() {
107107
@RequiresDialectFeature( value = DialectChecks.SupportsLockTimeouts.class,
108108
comment = "Test verifies proper exception throwing when a lock timeout is specified.",
109109
jiraKey = "HHH-7252" )
110-
@SkipForDialect(value = CockroachDialect.class, comment = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
111110
@SkipForDialect(value = AltibaseDialect.class, comment = "Altibase close socket after lock timeout occurred")
112111
public void testFindWithPessimisticWriteLockTimeoutException() {
113112
Lock lock = new Lock();
@@ -157,7 +156,6 @@ public void testFindWithPessimisticWriteLockTimeoutException() {
157156
@RequiresDialectFeature( value = DialectChecks.SupportsLockTimeouts.class,
158157
comment = "Test verifies proper exception throwing when a lock timeout is specified for Query#getSingleResult.",
159158
jiraKey = "HHH-13364" )
160-
@SkipForDialect(value = CockroachDialect.class, comment = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
161159
@SkipForDialect(value = AltibaseDialect.class, comment = "Altibase close socket after lock timeout occurred")
162160
public void testQuerySingleResultPessimisticWriteLockTimeoutException() {
163161
Lock lock = new Lock();
@@ -204,7 +202,6 @@ public void testQuerySingleResultPessimisticWriteLockTimeoutException() {
204202
@RequiresDialectFeature( value = DialectChecks.SupportsLockTimeouts.class,
205203
comment = "Test verifies proper exception throwing when a lock timeout is specified for Query#getResultList.",
206204
jiraKey = "HHH-13364" )
207-
@SkipForDialect(value = CockroachDialect.class, comment = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
208205
@SkipForDialect(value = AltibaseDialect.class, comment = "Altibase close socket after lock timeout occurred")
209206
public void testQueryResultListPessimisticWriteLockTimeoutException() {
210207
Lock lock = new Lock();
@@ -254,7 +251,6 @@ public void testQueryResultListPessimisticWriteLockTimeoutException() {
254251
@RequiresDialectFeature( value = DialectChecks.SupportsLockTimeouts.class,
255252
comment = "Test verifies proper exception throwing when a lock timeout is specified for NamedQuery#getResultList.",
256253
jiraKey = "HHH-13364" )
257-
@SkipForDialect(value = CockroachDialect.class, comment = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
258254
@SkipForDialect(value = AltibaseDialect.class, comment = "Altibase close socket after lock timeout occurred")
259255
public void testNamedQueryResultListPessimisticWriteLockTimeoutException() {
260256
Lock lock = new Lock();
@@ -298,7 +294,6 @@ public void testNamedQueryResultListPessimisticWriteLockTimeoutException() {
298294

299295
@Test
300296
@RequiresDialectFeature( value = DialectChecks.SupportsSkipLocked.class )
301-
@SkipForDialect(value = CockroachDialect.class, comment = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
302297
public void testUpdateWithPessimisticReadLockSkipLocked() {
303298
Lock lock = new Lock();
304299
lock.setName( "name" );
@@ -343,7 +338,6 @@ public void testUpdateWithPessimisticReadLockSkipLocked() {
343338

344339
@Test
345340
@RequiresDialectFeature(value = DialectChecks.SupportsLockTimeouts.class)
346-
@SkipForDialect(value = CockroachDialect.class, comment = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
347341
public void testUpdateWithPessimisticReadLockWithoutNoWait() {
348342
Lock lock = new Lock();
349343
lock.setName( "name" );

‎hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/StatementIsClosedAfterALockExceptionTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import org.hibernate.cfg.AvailableSettings;
1515
import org.hibernate.community.dialect.AltibaseDialect;
1616
import org.hibernate.community.dialect.InformixDialect;
17-
import org.hibernate.dialect.CockroachDialect;
1817
import org.hibernate.orm.test.jpa.BaseEntityManagerFunctionalTestCase;
1918
import org.hibernate.testing.orm.jdbc.PreparedStatementSpyConnectionProvider;
2019
import org.hibernate.testing.DialectChecks;
@@ -35,7 +34,6 @@
3534
* @author Andrea Boriero
3635
*/
3736
@RequiresDialectFeature({DialectChecks.SupportsLockTimeouts.class})
38-
@SkipForDialect(value = CockroachDialect.class, comment = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
3937
@SkipForDialect(value = AltibaseDialect.class, comment = "Altibase does not close Statement after lock timeout")
4038
public class StatementIsClosedAfterALockExceptionTest extends BaseEntityManagerFunctionalTestCase {
4139

‎hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockModeTest.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ public void prepareTest() throws Exception {
8787

8888
@Test
8989
@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class )
90-
@SkipForDialect(dialectClass = CockroachDialect.class, reason = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
9190
@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Can't commit transaction because Altibase closes socket after lock timeout")
9291
public void testLoading() {
9392
// open a session, begin a transaction and lock row
@@ -104,7 +103,6 @@ public void testLoading() {
104103

105104
@Test
106105
@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class )
107-
@SkipForDialect(dialectClass = CockroachDialect.class, reason = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
108106
@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Can't commit transaction because Altibase closes socket after lock timeout")
109107
public void testCriteria() {
110108
// open a session, begin a transaction and lock row
@@ -128,7 +126,6 @@ public void testCriteria() {
128126

129127
@Test
130128
@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class )
131-
@SkipForDialect(dialectClass = CockroachDialect.class, reason = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
132129
@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Can't commit transaction because Altibase closes socket after lock timeout")
133130
public void testCriteriaAliasSpecific() {
134131
// open a session, begin a transaction and lock row
@@ -154,7 +151,6 @@ public void testCriteriaAliasSpecific() {
154151

155152
@Test
156153
@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class )
157-
@SkipForDialect(dialectClass = CockroachDialect.class, reason = "for update clause does not imply locking. See https://github.com/cockroachdb/cockroach/issues/88995")
158154
@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Can't commit transaction because Altibase closes socket after lock timeout")
159155
public void testQuery() {
160156
// open a session, begin a transaction and lock row

‎hibernate-testing/src/main/java/org/hibernate/testing/orm/transaction/TransactionUtil.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.util.function.Function;
1111
import jakarta.persistence.EntityManager;
1212

13+
import jakarta.persistence.QueryTimeoutException;
1314
import org.hibernate.LockMode;
1415
import org.hibernate.LockOptions;
1516
import org.hibernate.SharedSessionContract;
@@ -172,7 +173,8 @@ public static void deleteRow(SessionFactoryScope factoryScope, String tableName,
172173
}
173174
catch (RuntimeException re) {
174175
if ( re.getCause() instanceof jakarta.persistence.LockTimeoutException
175-
|| re.getCause() instanceof org.hibernate.exception.LockTimeoutException ) {
176+
|| re.getCause() instanceof org.hibernate.exception.LockTimeoutException
177+
|| re.getCause() instanceof QueryTimeoutException ) {
176178
if ( !expectingToBlock ) {
177179
fail( "Expecting update to " + tableName + " to succeed, but failed due to async timeout (presumably due to locks)", re.getCause() );
178180
}
@@ -227,7 +229,8 @@ else if ( !expectingToBlock && resultSize == 0 ) {
227229
}
228230
catch (RuntimeException re) {
229231
if ( re.getCause() instanceof jakarta.persistence.LockTimeoutException
230-
|| re.getCause() instanceof org.hibernate.exception.LockTimeoutException ) {
232+
|| re.getCause() instanceof org.hibernate.exception.LockTimeoutException
233+
|| re.getCause() instanceof QueryTimeoutException ) {
231234
if ( !expectingToBlock ) {
232235
fail( "Expecting update to " + tableName + " to succeed, but failed due to async timeout (presumably due to locks)", re.getCause() );
233236
}

‎local-build-plugins/src/main/groovy/local.databases.gradle‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,8 @@ ext {
371371
'jdbc.url' : 'jdbc:postgresql://' + dbHost + ':26257/defaultdb?sslmode=disable&preparedStatementCacheQueries=0&escapeSyntaxCallMode=callIfNoReturn',
372372
'jdbc.datasource' : 'org.postgresql.Driver',
373373
// 'jdbc.datasource' : 'org.postgresql.ds.PGSimpleDataSource',
374-
'connection.init_sql' : ''
374+
// Configure the for-update clause to use proper locks: https://github.com/cockroachdb/cockroach/issues/88995
375+
'connection.init_sql' : 'set enable_durable_locking_for_serializable = on; set optimizer_use_lock_op_for_serializable = on;'
375376
],
376377
firebird : [
377378
'db.dialect' : 'org.hibernate.community.dialect.FirebirdDialect',

0 commit comments

Comments
 (0)