Skip to content

Commit 64ab956

Browse files
beikovsebersole
beikov
authored and
sebersole
committed
Improve locking test speed by using skip locked instead of waiting for timeout
1 parent f8fde28 commit 64ab956

File tree

7 files changed

+167
-76
lines changed

7 files changed

+167
-76
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public void testQueryLocking(SessionFactoryScope scope, boolean followOnLocking)
100100
statementInspector.assertExecutedCount( 1 );
101101
}
102102

103-
TransactionUtil.updateTable( scope, "employees", "salary", true );
103+
TransactionUtil.assertRowLock( scope, "employees", "salary", "id", employees.get( 0 ).getId(), true );
104104
}
105105
);
106106
} );

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,21 @@ void testSimpleUsage(SessionFactoryScope factoryScope) {
4444
final Timeout initialLockTimeout = connectionStrategy.getLockTimeout( conn, session.getFactory() );
4545
assertThat( initialLockTimeout.milliseconds() ).isEqualTo( expectedInitialValue );
4646

47-
final Timeout timeout = Timeout.milliseconds( 2000 );
48-
connectionStrategy.setLockTimeout( timeout, conn, session.getFactory() );
47+
try {
48+
final Timeout timeout = Timeout.milliseconds( 2000 );
49+
connectionStrategy.setLockTimeout( timeout, conn, session.getFactory() );
4950

50-
final Timeout adjustedLockTimeout = connectionStrategy.getLockTimeout( conn, session.getFactory() );
51-
assertThat( adjustedLockTimeout.milliseconds() ).isEqualTo( 2000 );
51+
final Timeout adjustedLockTimeout = connectionStrategy.getLockTimeout( conn, session.getFactory() );
52+
assertThat( adjustedLockTimeout.milliseconds() ).isEqualTo( 2000 );
5253

53-
connectionStrategy.setLockTimeout( Timeouts.WAIT_FOREVER, conn, session.getFactory() );
54+
connectionStrategy.setLockTimeout( Timeouts.WAIT_FOREVER, conn, session.getFactory() );
5455

55-
final Timeout resetLockTimeout = connectionStrategy.getLockTimeout( conn, session.getFactory() );
56-
assertThat( resetLockTimeout.milliseconds() ).isEqualTo( Timeouts.WAIT_FOREVER_MILLI );
56+
final Timeout resetLockTimeout = connectionStrategy.getLockTimeout( conn, session.getFactory() );
57+
assertThat( resetLockTimeout.milliseconds() ).isEqualTo( Timeouts.WAIT_FOREVER_MILLI );
58+
}
59+
finally {
60+
connectionStrategy.setLockTimeout( Timeout.milliseconds( expectedInitialValue ), conn, session.getFactory() );
61+
}
5762
} ) );
5863
}
5964

@@ -85,6 +90,13 @@ void testNoWait(SessionFactoryScope factoryScope) {
8590

8691
final ConnectionLockTimeoutStrategy connectionStrategy = lockingSupport.getConnectionLockTimeoutStrategy();
8792
final ConnectionLockTimeoutStrategy.Level lockTimeoutType = connectionStrategy.getSupportedLevel();
93+
final int initialValue;
94+
if ( session.getDialect() instanceof MySQLDialect ) {
95+
initialValue = 50;
96+
}
97+
else {
98+
initialValue = Timeouts.WAIT_FOREVER_MILLI;
99+
}
88100

89101
try {
90102
connectionStrategy.setLockTimeout( Timeouts.NO_WAIT, conn, session.getFactory() );
@@ -101,6 +113,9 @@ void testNoWait(SessionFactoryScope factoryScope) {
101113
throw e;
102114
}
103115
}
116+
finally {
117+
connectionStrategy.setLockTimeout( Timeout.milliseconds( initialValue ), conn, session.getFactory() );
118+
}
104119
} ) );
105120
}
106121
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,44 @@ public Report(Integer id, Person reporter, String... labels) {
5252
this.reporter = reporter;
5353
this.labels = Helper.toSet( labels );
5454
}
55+
56+
public Integer getId() {
57+
return id;
58+
}
59+
60+
public void setId(Integer id) {
61+
this.id = id;
62+
}
63+
64+
public Integer getRevision() {
65+
return revision;
66+
}
67+
68+
public void setRevision(Integer revision) {
69+
this.revision = revision;
70+
}
71+
72+
public String getTitle() {
73+
return title;
74+
}
75+
76+
public void setTitle(String title) {
77+
this.title = title;
78+
}
79+
80+
public Person getReporter() {
81+
return reporter;
82+
}
83+
84+
public void setReporter(Person reporter) {
85+
this.reporter = reporter;
86+
}
87+
88+
public Set<String> getLabels() {
89+
return labels;
90+
}
91+
92+
public void setLabels(Set<String> labels) {
93+
this.labels = labels;
94+
}
5595
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ void simpleTest(SessionFactoryScope factoryScope) {
4848

4949
session.clear();
5050
sqlCollector.clear();
51-
session.find( Detail.class, 1, LockModeType.PESSIMISTIC_WRITE );
51+
final Detail detail = session.find( Detail.class, 1, LockModeType.PESSIMISTIC_WRITE );
5252
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
5353
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), Tables.DETAILS, Tables.SUPPLEMENTALS );
54-
TransactionUtil.updateTable( factoryScope, Tables.DETAILS.getTableName(), "name", true );
55-
TransactionUtil.updateTable( factoryScope, Tables.SUPPLEMENTALS.getTableName(), "txt", true );
54+
TransactionUtil.assertRowLock( factoryScope, Tables.DETAILS.getTableName(), "name", "id", detail.getId(), true );
55+
TransactionUtil.assertRowLock( factoryScope, Tables.SUPPLEMENTALS.getTableName(), "txt", "detail_fk", detail.getId(), true );
5656
} );
5757
}
5858

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

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ void testFind(SessionFactoryScope factoryScope) {
6969
assertThat( Hibernate.isInitialized( theTalisman ) ).isTrue();
7070
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
7171
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), BOOKS );
72-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
73-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", false );
72+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
73+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), false );
7474
} );
7575
}
7676

@@ -85,9 +85,9 @@ void testFindWithExtended(SessionFactoryScope factoryScope) {
8585
assertThat( Hibernate.isInitialized( theTalisman ) ).isTrue();
8686
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
8787
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), BOOKS );
88-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
88+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
8989
// For strict compliance, EXTENDED here should lock `book_genres` but we do not
90-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", false );
90+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), false );
9191
} );
9292
}
9393

@@ -103,8 +103,8 @@ void testFindWithExtendedJpaExpectation(SessionFactoryScope factoryScope) {
103103
// these 2 assertions would depend a bit on the approach and/or dialect
104104
// assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
105105
// Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), Helper.Table.BOOK_GENRES );
106-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
107-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", true );
106+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
107+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), true );
108108
} );
109109
}
110110

@@ -131,8 +131,8 @@ void testFindWithExtendedAndFetch(SessionFactoryScope factoryScope) {
131131
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
132132
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), BOOKS, BOOK_GENRES );
133133

134-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
135-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", true );
134+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
135+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), true );
136136
}
137137
else {
138138
// should be 3, but follow-on locking is not locking collection tables...
@@ -142,7 +142,7 @@ void testFindWithExtendedAndFetch(SessionFactoryScope factoryScope) {
142142

143143
// todo : track this down - HHH-19513
144144
//Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), BOOK_GENRES );
145-
//TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", true );
145+
//TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), true );
146146
}
147147
} );
148148
}
@@ -161,8 +161,8 @@ void testLock(SessionFactoryScope factoryScope) {
161161

162162
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), BOOKS );
163163

164-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
165-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", false );
164+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
165+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), false );
166166
} );
167167
}
168168

@@ -178,9 +178,9 @@ void testLockWithExtended(SessionFactoryScope factoryScope) {
178178
session.lock( theTalisman, PESSIMISTIC_WRITE, EXTENDED );
179179
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
180180
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), BOOKS );
181-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
181+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
182182
// Again, for strict compliance, EXTENDED here should lock `book_genres` but we do not
183-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", false );
183+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), false );
184184
} );
185185
}
186186

@@ -196,8 +196,8 @@ void testRefresh(SessionFactoryScope factoryScope) {
196196
session.refresh( theTalisman, PESSIMISTIC_WRITE );
197197
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
198198
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), BOOKS );
199-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
200-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", false );
199+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
200+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), false );
201201
} );
202202
}
203203

@@ -213,9 +213,9 @@ void testRefreshWithExtended(SessionFactoryScope factoryScope) {
213213
session.refresh( theTalisman, PESSIMISTIC_WRITE, EXTENDED );
214214
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
215215
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), BOOKS );
216-
TransactionUtil.updateTable( factoryScope, BOOKS.getTableName(), "title", true );
216+
TransactionUtil.assertRowLock( factoryScope, BOOKS.getTableName(), "title", "id", theTalisman.getId(), true );
217217
// Again, for strict compliance, EXTENDED here should lock `book_genres` but we do not
218-
TransactionUtil.updateTable( factoryScope, BOOK_GENRES.getTableName(), "genre", false );
218+
TransactionUtil.assertRowLock( factoryScope, BOOK_GENRES.getTableName(), "genre", "book_fk", theTalisman.getId(), false );
219219
} );
220220
}
221221

@@ -226,12 +226,12 @@ void testEagerFind(SessionFactoryScope factoryScope) {
226226

227227
factoryScope.inTransaction( (session) -> {
228228
sqlCollector.clear();
229-
session.find( Report.class, 2, PESSIMISTIC_WRITE );
229+
final Report report = session.find( Report.class, 2, PESSIMISTIC_WRITE );
230230
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
231231
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), REPORTS );
232-
TransactionUtil.updateTable( factoryScope, REPORTS.getTableName(), "title", true );
233-
TransactionUtil.updateTable( factoryScope, REPORT_LABELS.getTableName(), "txt", willAggressivelyLockJoinedTables( session.getDialect() ) );
234-
TransactionUtil.updateTable( factoryScope, PERSONS.getTableName(), "name", willAggressivelyLockJoinedTables( session.getDialect() ) );
232+
TransactionUtil.assertRowLock( factoryScope, REPORTS.getTableName(), "title", "id", report.getId(), true );
233+
TransactionUtil.assertRowLock( factoryScope, REPORT_LABELS.getTableName(), "txt", "report_fk", report.getId(), willAggressivelyLockJoinedTables( session.getDialect() ) );
234+
TransactionUtil.assertRowLock( factoryScope, PERSONS.getTableName(), "name", "id", report.getReporter().getId(), willAggressivelyLockJoinedTables( session.getDialect() ) );
235235
} );
236236
}
237237

@@ -258,28 +258,29 @@ void testEagerFindWithExtended(SessionFactoryScope factoryScope) {
258258

259259
factoryScope.inTransaction( (session) -> {
260260
sqlCollector.clear();
261-
session.find( Report.class, 2, PESSIMISTIC_WRITE, EXTENDED );
261+
final Report report = session.find( Report.class, 2, PESSIMISTIC_WRITE, EXTENDED );
262262
if ( session.getDialect().supportsOuterJoinForUpdate() ) {
263263
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
264264
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), REPORTS, REPORT_LABELS );
265-
TransactionUtil.updateTable( factoryScope, REPORTS.getTableName(), "title", true );
266-
TransactionUtil.updateTable( factoryScope, PERSONS.getTableName(), "name", willAggressivelyLockJoinedTables( session.getDialect() ) );
267-
TransactionUtil.updateTable( factoryScope, REPORT_LABELS.getTableName(), "txt", true );
265+
TransactionUtil.assertRowLock( factoryScope, REPORTS.getTableName(), "title", "id", report.getId(), true );
266+
TransactionUtil.assertRowLock( factoryScope, PERSONS.getTableName(), "name", "id", report.getReporter().getId(),
267+
willAggressivelyLockJoinedTables( session.getDialect() ) );
268+
TransactionUtil.assertRowLock( factoryScope, REPORT_LABELS.getTableName(), "txt", "report_fk", report.getId(), true );
268269
}
269270
else {
270271
assertThat( sqlCollector.getSqlQueries() ).hasSize( 3 );
271272
Helper.checkSql( sqlCollector.getSqlQueries().get( 1 ), session.getDialect(), REPORTS );
272273
Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), PERSONS );
273-
TransactionUtil.updateTable( factoryScope, REPORTS.getTableName(), "title", true );
274+
TransactionUtil.assertRowLock( factoryScope, REPORTS.getTableName(), "title", "id", report.getId(), true );
274275

275276
// these should happen but currently do not - follow-on locking is not locking element-collection tables...
276277
// todo : track this down - HHH-19513
277278
//Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), REPORT_LABELS );
278-
//TransactionUtil.updateTable( factoryScope, REPORT_LABELS.getTableName(), "txt", true );
279+
//TransactionUtil.assertRowLock( factoryScope, REPORT_LABELS.getTableName(), "txt", "report_fk", report.getId(), true );
279280

280281
// this one should not happen at all. follow-on locking is not understanding scope probably..
281282
// todo : track this down - HHH-19514
282-
TransactionUtil.updateTable( factoryScope, PERSONS.getTableName(), "name", true );
283+
TransactionUtil.assertRowLock( factoryScope, PERSONS.getTableName(), "name", "id", report.getReporter().getId(), true );
283284
}
284285
} );
285286
}
@@ -292,29 +293,29 @@ void testEagerFindWithFetchScope(SessionFactoryScope factoryScope) {
292293

293294
factoryScope.inTransaction( (session) -> {
294295
sqlCollector.clear();
295-
session.find( Report.class, 2, PESSIMISTIC_WRITE, Locking.Scope.INCLUDE_FETCHES );
296+
final Report report = session.find( Report.class, 2, PESSIMISTIC_WRITE, Locking.Scope.INCLUDE_FETCHES );
296297

297298
if ( session.getDialect().supportsOuterJoinForUpdate() ) {
298299
assertThat( sqlCollector.getSqlQueries() ).hasSize( 1 );
299300
Helper.checkSql( sqlCollector.getSqlQueries().get( 0 ), session.getDialect(), REPORTS, REPORT_LABELS, JOINED_REPORTER );
300-
TransactionUtil.updateTable( factoryScope, REPORTS.getTableName(), "title", true );
301-
TransactionUtil.updateTable( factoryScope, PERSONS.getTableName(), "name", true );
302-
TransactionUtil.updateTable( factoryScope, REPORT_LABELS.getTableName(), "txt", true );
301+
TransactionUtil.assertRowLock( factoryScope, REPORTS.getTableName(), "title", "id", report.getId(), true );
302+
TransactionUtil.assertRowLock( factoryScope, PERSONS.getTableName(), "name", "id", report.getReporter().getId(), true );
303+
TransactionUtil.assertRowLock( factoryScope, REPORT_LABELS.getTableName(), "txt", "report_fk", report.getId(), true );
303304
}
304305
else {
305306
assertThat( sqlCollector.getSqlQueries() ).hasSize( 3 );
306307
Helper.checkSql( sqlCollector.getSqlQueries().get( 1 ), session.getDialect(), REPORTS );
307308
Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), PERSONS );
308-
TransactionUtil.updateTable( factoryScope, REPORTS.getTableName(), "title", true );
309+
TransactionUtil.assertRowLock( factoryScope, REPORTS.getTableName(), "title", "id", report.getId(), true );
309310

310311
// these should happen but currently do not - follow-on locking is not locking element-collection tables...
311312
// todo : track this down - HHH-19513
312313
//Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), REPORT_LABELS );
313-
//TransactionUtil.updateTable( factoryScope, REPORT_LABELS.getTableName(), "txt", true );
314+
//TransactionUtil.assertRowLock( factoryScope, REPORT_LABELS.getTableName(), "txt", "report_fk", report.getId(), true );
314315

315316
// this one should not happen at all. follow-on locking is not understanding scope probably..
316317
// todo : track this down - HHH-19514
317-
TransactionUtil.updateTable( factoryScope, PERSONS.getTableName(), "name", true );
318+
TransactionUtil.assertRowLock( factoryScope, PERSONS.getTableName(), "name", "id", report.getReporter().getId(), true );
318319
}
319320
} );
320321
}

‎hibernate-testing/src/main/java/org/hibernate/testing/orm/AsyncExecutor.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,34 +7,42 @@
77
import java.util.concurrent.ExecutionException;
88
import java.util.concurrent.ExecutorService;
99
import java.util.concurrent.Executors;
10+
import java.util.concurrent.Future;
1011
import java.util.concurrent.TimeUnit;
1112

1213
/**
1314
* @author Steve Ebersole
1415
*/
1516
public class AsyncExecutor {
17+
18+
// Need more than a single thread, because not all databases support cancellation of statements waiting for locks
19+
private static final ExecutorService EXECUTOR_SERVICE = Executors.newCachedThreadPool();
20+
1621
public static void executeAsync(Runnable action) {
17-
final ExecutorService executorService = Executors.newSingleThreadExecutor();
22+
final Future<?> future = EXECUTOR_SERVICE.submit( action );
1823
try {
19-
executorService.submit( action ).get();
24+
future.get();
2025
}
2126
catch (InterruptedException e) {
22-
throw new RuntimeException( "Thread interruption", e );
27+
future.cancel( true );
28+
throw new TimeoutException( "Thread interruption", e );
2329
}
2430
catch (ExecutionException e) {
25-
throw new RuntimeException( "Async execution error", e );
31+
throw new RuntimeException( "Async execution error", e.getCause() );
2632
}
2733
}
2834

2935
public static void executeAsync(int timeout, TimeUnit timeoutUnit, Runnable action) {
30-
final ExecutorService executorService = Executors.newSingleThreadExecutor();
36+
final Future<?> future = EXECUTOR_SERVICE.submit( action );
3137
try {
32-
executorService.submit( action ).get( timeout, timeoutUnit );
38+
future.get( timeout, timeoutUnit );
3339
}
3440
catch (InterruptedException e) {
41+
future.cancel( true );
3542
throw new TimeoutException( "Thread interruption", e );
3643
}
3744
catch (java.util.concurrent.TimeoutException e) {
45+
future.cancel( true );
3846
throw new TimeoutException( "Thread timeout exceeded", e );
3947
}
4048
catch (ExecutionException e) {

0 commit comments

Comments
 (0)