diff --git a/README.md b/README.md index 6184857..66d86e1 100755 --- a/README.md +++ b/README.md @@ -343,3 +343,4 @@ Modules with their own `pom.xml` that run plain Hibernate without the Spring Boo |---|---| | [`h8-preview`](h8-preview/) | Hibernate ORM 8 beta against Hibernate 7: what breaks | | [`inheritance`](inheritance/) | `SINGLE_TABLE` vs `JOINED` vs `TABLE_PER_CLASS` vs `@MappedSuperclass`: generated DDL, SQL, failures and timings (Hibernate `7.4.13.Final`) | +| [`locking`](locking/) | Optimistic (`@Version`, retries) vs pessimistic (`PESSIMISTIC_WRITE`, `NOWAIT`, `SKIP LOCKED`) locking, lost updates and deadlocks on H2 (Hibernate `7.4.14.Final`) | diff --git a/locking/README.md b/locking/README.md new file mode 100644 index 0000000..b02da7f --- /dev/null +++ b/locking/README.md @@ -0,0 +1,36 @@ +# locking + +Companion module for the ankurm.com article on **optimistic vs pessimistic locking in Hibernate 7**. Two threads, one +in-memory H2 database, plain Hibernate (no Spring). Each test writes what it observed to `output/`. + +## Versions + +| Component | Version | Notes | +|---|---|---| +| Hibernate ORM | `7.4.14.Final` | latest 7.x GA on Maven Central on 2026-10-25 (`8.0.0.Beta3` is also listed; it is not GA) | +| Jakarta Persistence | `3.2.0` | | +| H2 | `2.4.240` | in memory, `OPTIMIZE_REUSE_RESULTS=0` (see `Db.java`) | +| JUnit Jupiter / AssertJ | `6.0.3` / `3.27.7` | | +| Java | `25` (Temurin 25.0.5+8) | | + +This module has its own `pom.xml` and does not use the Spring Boot parent of the main `hibernate-demo` project. +All locking behaviour here is H2's. `SKIP LOCKED`, `NOWAIT` and deadlock handling differ between databases, so check +your own. + +## Run + +```bash +./scripts/run-all.sh +``` + +## Output + +| File | What it shows | +|---|---| +| `01-lost-update.txt` | two transactions read 100, both deposit, one deposit is lost (no `@Version`) | +| `02-version-stops-it.txt` | the same race with `@Version`: the second commit fails with `OptimisticLockException` | +| `03-retry-loop.txt` | six threads, thirty increments, retry on `OptimisticLockException`; the final balance is exact | +| `04-pessimistic-write.txt` | `PESSIMISTIC_WRITE` makes the second transaction wait; SQL uses `for update` | +| `05-no-wait.txt` | `Timeouts.NO_WAIT` fails at once (`for update nowait`) | +| `06-skip-locked.txt` | two workers claim different tickets (`for update skip locked`) | +| `07-deadlock.txt` | opposite lock order deadlocks; the same order for everyone does not | diff --git a/locking/output/01-lost-update.txt b/locking/output/01-lost-update.txt new file mode 100644 index 0000000..a59b652 --- /dev/null +++ b/locking/output/01-lost-update.txt @@ -0,0 +1,7 @@ +start balance 100; transaction A deposits 10, transaction B deposits 20 +both read 100 before either writes; A commits first, then B +final balance: 120 (a correct result would be 130) + +update statements Hibernate sent: +update Account set balance=?,owner=? where id=? +update Account set balance=?,owner=? where id=? diff --git a/locking/output/02-version-stops-it.txt b/locking/output/02-version-stops-it.txt new file mode 100644 index 0000000..2eaa4d7 --- /dev/null +++ b/locking/output/02-version-stops-it.txt @@ -0,0 +1,9 @@ +same race, entity has @Version +A: committed. B: failed +B exception: jakarta.persistence.OptimisticLockException +caused by: org.hibernate.StaleObjectStateException +final balance: 110, version: 1 + +update statements Hibernate sent: +update VersionedAccount set balance=?,owner=?,version=? where id=? and version=? +update VersionedAccount set balance=?,owner=?,version=? where id=? and version=? diff --git a/locking/output/03-retry-loop.txt b/locking/output/03-retry-loop.txt new file mode 100644 index 0000000..d17f771 --- /dev/null +++ b/locking/output/03-retry-loop.txt @@ -0,0 +1,5 @@ +6 threads x 5 deposits of 1, each retried on OptimisticLockException +final balance: 30 (expected 30) +version: 30 +retries happened: true +retry count in this run: 27 (varies from run to run) diff --git a/locking/output/04-pessimistic-write.txt b/locking/output/04-pessimistic-write.txt new file mode 100644 index 0000000..a5c8790 --- /dev/null +++ b/locking/output/04-pessimistic-write.txt @@ -0,0 +1,8 @@ +A locks row 1 with PESSIMISTIC_WRITE, holds it 600 ms, deposits 10 +B asks for the same lock, then deposits 20 +B waited at least 400 ms for the lock: true +final balance: 130 (correct: 130) + +select statements Hibernate sent (one per transaction): +select a1_0.id,a1_0.balance,a1_0.owner from Account a1_0 where a1_0.id=? for update +select a1_0.id,a1_0.balance,a1_0.owner from Account a1_0 where a1_0.id=? for update diff --git a/locking/output/05-no-wait.txt b/locking/output/05-no-wait.txt new file mode 100644 index 0000000..b2b2d37 --- /dev/null +++ b/locking/output/05-no-wait.txt @@ -0,0 +1,8 @@ +A holds the lock; B asks with Timeouts.NO_WAIT +B exception: org.hibernate.exception.LockTimeoutException + caused by org.h2.jdbc.JdbcSQLTimeoutException + H2 message: Timeout trying to lock table "ACCOUNT"; SQL statement: + caused by org.h2.mvstore.MVStoreException + +select statements B sent: +select a1_0.id,a1_0.balance,a1_0.owner from Account a1_0 where a1_0.id=? for update nowait diff --git a/locking/output/06-skip-locked.txt b/locking/output/06-skip-locked.txt new file mode 100644 index 0000000..5afba06 --- /dev/null +++ b/locking/output/06-skip-locked.txt @@ -0,0 +1,7 @@ +5 tickets, all NEW. Worker A claims 2 and keeps its transaction open. +Worker B claims 2 with SKIP LOCKED. +A got tickets: [1, 2] +B got tickets: [3, 4] + +select statements B sent: +select t1_0.id,t1_0.claimedBy,t1_0.status from Ticket t1_0 where t1_0.status='NEW' order by t1_0.id fetch first ? rows only for update skip locked diff --git a/locking/output/07-deadlock.txt b/locking/output/07-deadlock.txt new file mode 100644 index 0000000..4ecb0cf --- /dev/null +++ b/locking/output/07-deadlock.txt @@ -0,0 +1,10 @@ +opposite lock order (A: 1 then 2, B: 2 then 1) +transactions that failed: 1 + jakarta.persistence.PessimisticLockException + caused by org.hibernate.exception.LockAcquisitionException + caused by org.h2.jdbc.JdbcSQLTransactionRollbackException + H2 message: Deadlock detected. The current transaction was rolled back. Details: "ACCOUNT"; SQL statement: + caused by org.h2.mvstore.MVStoreException + +same lock order for both (always the lower id first) +transactions that failed: 0 diff --git a/locking/pom.xml b/locking/pom.xml new file mode 100644 index 0000000..ed02661 --- /dev/null +++ b/locking/pom.xml @@ -0,0 +1,57 @@ + + + 4.0.0 + + com.ankurm + locking + 1.0.0 + locking + Hibernate 7 optimistic vs pessimistic locking: lost updates, @Version, retries, PESSIMISTIC_WRITE, SKIP LOCKED and deadlocks, run with plain Hibernate (no Spring) + + + 25 + UTF-8 + 7.4.14.Final + + + + + org.hibernate.orm + hibernate-core + ${hibernate.version} + + + jakarta.persistence + jakarta.persistence-api + 3.2.0 + + + com.h2database + h2 + 2.4.240 + + + org.junit.jupiter + junit-jupiter + 6.0.3 + test + + + org.assertj + assertj-core + 3.27.7 + test + + + + + + + maven-surefire-plugin + 3.6.0 + + + + diff --git a/locking/scripts/run-all.sh b/locking/scripts/run-all.sh new file mode 100755 index 0000000..a026be8 --- /dev/null +++ b/locking/scripts/run-all.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +# Regenerates every file under output/. Needs JDK 25 and Maven 3.9 with access to Maven Central. +# The tests assert the facts they print. The only figure that changes between runs is the retry count in +# output/03-retry-loop.txt. +set -euo pipefail +cd "$(dirname "$0")/.." +mvn -B clean test +ls -1 output diff --git a/locking/src/main/java/com/ankurm/locking/Account.java b/locking/src/main/java/com/ankurm/locking/Account.java new file mode 100644 index 0000000..747fe3a --- /dev/null +++ b/locking/src/main/java/com/ankurm/locking/Account.java @@ -0,0 +1,23 @@ +package com.ankurm.locking; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; + +/** No version column: two transactions can silently overwrite each other. */ +@Entity +public class Account { + + @Id + public Long id; + public String owner; + public long balance; + + protected Account() { + } + + public Account(Long id, String owner, long balance) { + this.id = id; + this.owner = owner; + this.balance = balance; + } +} diff --git a/locking/src/main/java/com/ankurm/locking/Ticket.java b/locking/src/main/java/com/ankurm/locking/Ticket.java new file mode 100644 index 0000000..d2002e8 --- /dev/null +++ b/locking/src/main/java/com/ankurm/locking/Ticket.java @@ -0,0 +1,22 @@ +package com.ankurm.locking; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; + +/** A unit of work in a database-backed queue. */ +@Entity +public class Ticket { + + @Id + public Long id; + public String status; + public String claimedBy; + + protected Ticket() { + } + + public Ticket(Long id, String status) { + this.id = id; + this.status = status; + } +} diff --git a/locking/src/main/java/com/ankurm/locking/VersionedAccount.java b/locking/src/main/java/com/ankurm/locking/VersionedAccount.java new file mode 100644 index 0000000..f766c5f --- /dev/null +++ b/locking/src/main/java/com/ankurm/locking/VersionedAccount.java @@ -0,0 +1,27 @@ +package com.ankurm.locking; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Version; + +/** Same shape as {@link Account} plus a {@code @Version} column. */ +@Entity +public class VersionedAccount { + + @Id + public Long id; + public String owner; + public long balance; + + @Version + public long version; + + protected VersionedAccount() { + } + + public VersionedAccount(Long id, String owner, long balance) { + this.id = id; + this.owner = owner; + this.balance = balance; + } +} diff --git a/locking/src/test/java/com/ankurm/locking/Db.java b/locking/src/test/java/com/ankurm/locking/Db.java new file mode 100644 index 0000000..2d30e43 --- /dev/null +++ b/locking/src/test/java/com/ankurm/locking/Db.java @@ -0,0 +1,93 @@ +package com.ankurm.locking; + +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.resource.jdbc.spi.StatementInspector; +import org.hibernate.SessionFactory; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +/** Plain-Hibernate bootstrap against in-memory H2, recording every SQL string Hibernate prepares. */ +final class Db implements AutoCloseable { + + private static final AtomicInteger SEQ = new AtomicInteger(); + + /** Every statement Hibernate prepared since the last {@link #clear()}. */ + final List sql = java.util.Collections.synchronizedList(new ArrayList<>()); + final SessionFactory sessionFactory; + private final String url; + + Db(Class... entities) { + // OPTIMIZE_REUSE_RESULTS=0: by default H2 may hand back the previous result of an identical query when no + // table changed. Switched off so every query in these tests really reaches the table. + this.url = "jdbc:h2:mem:lck" + SEQ.incrementAndGet() + ";DB_CLOSE_DELAY=-1;OPTIMIZE_REUSE_RESULTS=0"; + StatementInspector inspector = s -> { + sql.add(s); + return s; + }; + StandardServiceRegistry registry = new StandardServiceRegistryBuilder() + .applySetting(AvailableSettings.JAKARTA_JDBC_URL, url) + .applySetting(AvailableSettings.JAKARTA_JDBC_USER, "sa") + .applySetting(AvailableSettings.JAKARTA_JDBC_PASSWORD, "") + .applySetting(AvailableSettings.HBM2DDL_AUTO, "create-drop") + .applySetting(AvailableSettings.STATEMENT_INSPECTOR, inspector) + .build(); + MetadataSources sources = new MetadataSources(registry); + for (Class e : entities) { + sources.addAnnotatedClass(e); + } + this.sessionFactory = sources.buildMetadata().buildSessionFactory(); + } + + void clear() { + sql.clear(); + } + + /** Raw JDBC against the same in-memory database. */ + List jdbc(String query) { + List rows = new ArrayList<>(); + try (Connection c = java.sql.DriverManager.getConnection(url, "sa", ""); + Statement st = c.createStatement(); + ResultSet rs = st.executeQuery(query)) { + int cols = rs.getMetaData().getColumnCount(); + while (rs.next()) { + StringBuilder sb = new StringBuilder(); + for (int i = 1; i <= cols; i++) { + if (i > 1) sb.append(" | "); + sb.append(rs.getString(i)); + } + rows.add(sb.toString()); + } + } catch (SQLException e) { + throw new IllegalStateException(e); + } + return rows; + } + + /** Raw JDBC statement; returns the database's error message if it is rejected, or "accepted". */ + String tryExec(String statement) { + try (Connection c = java.sql.DriverManager.getConnection(url, "sa", ""); + Statement st = c.createStatement()) { + st.executeUpdate(statement); + return "accepted"; + } catch (SQLException e) { + return e.getMessage(); + } + } + + @Override + public void close() { + sessionFactory.close(); + } +} diff --git a/locking/src/test/java/com/ankurm/locking/OptimisticTest.java b/locking/src/test/java/com/ankurm/locking/OptimisticTest.java new file mode 100644 index 0000000..bb37f6c --- /dev/null +++ b/locking/src/test/java/com/ankurm/locking/OptimisticTest.java @@ -0,0 +1,162 @@ +package com.ankurm.locking; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicInteger; + +import jakarta.persistence.OptimisticLockException; +import org.junit.jupiter.api.Test; + +/** Lost update without a version, the same race with @Version, and the retry loop that makes it usable. */ +class OptimisticTest { + + @Test + void lostUpdateWithoutVersion() { + try (Db db = new Db(Account.class); Transcript t = new Transcript("01-lost-update")) { + db.sessionFactory.inTransaction(s -> s.persist(new Account(1L, "alice", 100))); + db.clear(); + + CountDownLatch bothRead = new CountDownLatch(2); + CountDownLatch firstCommitted = new CountDownLatch(1); + ExecutorService pool = Executors.newFixedThreadPool(2); + Future a = pool.submit(() -> db.sessionFactory.inTransaction(s -> { + Account acc = s.find(Account.class, 1L); + bothRead.countDown(); + await(bothRead); + acc.balance += 10; + })); + Future b = pool.submit(() -> { + db.sessionFactory.inTransaction(s -> { + Account acc = s.find(Account.class, 1L); + bothRead.countDown(); + await(bothRead); + await(firstCommitted); + acc.balance += 20; + }); + }); + get(a); + firstCommitted.countDown(); + get(b); + pool.shutdown(); + + long balance = db.sessionFactory.fromSession(s -> s.find(Account.class, 1L).balance); + t.line("start balance 100; transaction A deposits 10, transaction B deposits 20"); + t.line("both read 100 before either writes; A commits first, then B"); + t.line("final balance: " + balance + " (a correct result would be 130)"); + t.blank().line("update statements Hibernate sent:"); + db.sql.stream().filter(x -> x.startsWith("update")).forEach(t::line); + assertThat(balance).isEqualTo(120); + } + } + + @Test + void versionStopsTheLostUpdate() { + try (Db db = new Db(VersionedAccount.class); Transcript t = new Transcript("02-version-stops-it")) { + db.sessionFactory.inTransaction(s -> s.persist(new VersionedAccount(1L, "alice", 100))); + db.clear(); + + CountDownLatch bothRead = new CountDownLatch(2); + CountDownLatch firstCommitted = new CountDownLatch(1); + ExecutorService pool = Executors.newFixedThreadPool(2); + Future a = pool.submit(() -> db.sessionFactory.inTransaction(s -> { + VersionedAccount acc = s.find(VersionedAccount.class, 1L); + bothRead.countDown(); + await(bothRead); + acc.balance += 10; + })); + Future b = pool.submit(() -> { + try { + db.sessionFactory.inTransaction(s -> { + VersionedAccount acc = s.find(VersionedAccount.class, 1L); + bothRead.countDown(); + await(bothRead); + await(firstCommitted); + acc.balance += 20; + }); + return null; + } catch (Throwable e) { + return e; + } + }); + get(a); + firstCommitted.countDown(); + Throwable failure; + try { + failure = b.get(); + } catch (Exception e) { + throw new IllegalStateException(e); + } + pool.shutdown(); + + VersionedAccount after = db.sessionFactory.fromSession(s -> s.find(VersionedAccount.class, 1L)); + t.line("same race, entity has @Version"); + t.line("A: committed. B: " + (failure == null ? "committed" : "failed")); + t.line("B exception: " + failure.getClass().getName()); + t.line("caused by: " + failure.getCause().getClass().getName()); + t.line("final balance: " + after.balance + ", version: " + after.version); + t.blank().line("update statements Hibernate sent:"); + db.sql.stream().filter(x -> x.startsWith("update")).forEach(t::line); + assertThat(failure).isInstanceOf(OptimisticLockException.class); + assertThat(after.balance).isEqualTo(110); + assertThat(after.version).isEqualTo(1); + } + } + + @Test + void retryLoopConverges() { + try (Db db = new Db(VersionedAccount.class); Transcript t = new Transcript("03-retry-loop")) { + db.sessionFactory.inTransaction(s -> s.persist(new VersionedAccount(1L, "alice", 0))); + int threads = 6; + int depositsEach = 5; + AtomicInteger retries = new AtomicInteger(); + ExecutorService pool = Executors.newFixedThreadPool(threads); + List> futures = new ArrayList<>(); + for (int i = 0; i < threads; i++) { + futures.add(pool.submit(() -> { + for (int d = 0; d < depositsEach; d++) { + while (true) { + try { + db.sessionFactory.inTransaction(s -> s.find(VersionedAccount.class, 1L).balance += 1); + break; + } catch (OptimisticLockException e) { + retries.incrementAndGet(); + } + } + } + })); + } + futures.forEach(OptimisticTest::get); + pool.shutdown(); + VersionedAccount after = db.sessionFactory.fromSession(s -> s.find(VersionedAccount.class, 1L)); + t.line(threads + " threads x " + depositsEach + " deposits of 1, each retried on OptimisticLockException"); + t.line("final balance: " + after.balance + " (expected " + threads * depositsEach + ")"); + t.line("version: " + after.version); + t.line("retries happened: " + (retries.get() > 0)); + t.line("retry count in this run: " + retries.get() + " (varies from run to run)"); + assertThat(after.balance).isEqualTo(threads * depositsEach); + assertThat(after.version).isEqualTo(threads * depositsEach); + } + } + + static void await(CountDownLatch l) { + try { + l.await(); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + + static void get(Future f) { + try { + f.get(); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } +} diff --git a/locking/src/test/java/com/ankurm/locking/PessimisticTest.java b/locking/src/test/java/com/ankurm/locking/PessimisticTest.java new file mode 100644 index 0000000..80e09d5 --- /dev/null +++ b/locking/src/test/java/com/ankurm/locking/PessimisticTest.java @@ -0,0 +1,223 @@ +package com.ankurm.locking; + +import static com.ankurm.locking.OptimisticTest.await; +import static com.ankurm.locking.OptimisticTest.get; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import jakarta.persistence.LockModeType; +import org.hibernate.Timeouts; +import org.junit.jupiter.api.Test; + +/** PESSIMISTIC_WRITE: waiting, giving up, skipping locked rows, and a deadlock. */ +class PessimisticTest { + + private static String describe(Throwable e) { + StringBuilder sb = new StringBuilder(e.getClass().getName()); + for (Throwable c = e.getCause(); c != null; c = c.getCause()) { + sb.append("\n caused by ").append(c.getClass().getName()); + if (c.getClass().getName().startsWith("org.h2.jdbc")) { + sb.append("\n H2 message: ").append(c.getMessage().lines().findFirst().orElse("")); + } + } + return sb.toString(); + } + + @Test + void secondWriterWaitsForTheFirst() { + try (Db db = new Db(Account.class); Transcript t = new Transcript("04-pessimistic-write")) { + db.sessionFactory.inTransaction(s -> s.persist(new Account(1L, "alice", 100))); + db.clear(); + CountDownLatch locked = new CountDownLatch(1); + ExecutorService pool = Executors.newFixedThreadPool(2); + Future a = pool.submit(() -> db.sessionFactory.inTransaction(s -> { + Account acc = s.find(Account.class, 1L, LockModeType.PESSIMISTIC_WRITE); + locked.countDown(); + sleep(600); + acc.balance += 10; + })); + long[] waitedMillis = new long[1]; + Future b = pool.submit(() -> { + await(locked); + long start = System.nanoTime(); + db.sessionFactory.inTransaction(s -> { + Account acc = s.find(Account.class, 1L, LockModeType.PESSIMISTIC_WRITE); + waitedMillis[0] = (System.nanoTime() - start) / 1_000_000; + acc.balance += 20; + }); + }); + get(a); + get(b); + pool.shutdown(); + List sent = new ArrayList<>(db.sql); + long balance = db.sessionFactory.fromSession(s -> s.find(Account.class, 1L).balance); + t.line("A locks row 1 with PESSIMISTIC_WRITE, holds it 600 ms, deposits 10"); + t.line("B asks for the same lock, then deposits 20"); + t.line("B waited at least 400 ms for the lock: " + (waitedMillis[0] >= 400)); + t.line("final balance: " + balance + " (correct: 130)"); + t.blank().line("select statements Hibernate sent (one per transaction):"); + sent.stream().filter(x -> x.startsWith("select")).forEach(t::line); + assertThat(waitedMillis[0]).isGreaterThanOrEqualTo(400); + assertThat(balance).isEqualTo(130); + } + } + + @Test + void noWaitGivesUpImmediately() { + try (Db db = new Db(Account.class); Transcript t = new Transcript("05-no-wait")) { + db.sessionFactory.inTransaction(s -> s.persist(new Account(1L, "alice", 100))); + db.clear(); + CountDownLatch locked = new CountDownLatch(1); + CountDownLatch done = new CountDownLatch(1); + ExecutorService pool = Executors.newFixedThreadPool(2); + Future a = pool.submit(() -> db.sessionFactory.inTransaction(s -> { + s.find(Account.class, 1L, LockModeType.PESSIMISTIC_WRITE); + locked.countDown(); + await(done); + })); + await(locked); + db.clear(); + Throwable failure = null; + try { + db.sessionFactory.inTransaction(s -> + s.find(Account.class, 1L, LockModeType.PESSIMISTIC_WRITE, Timeouts.NO_WAIT)); + } catch (Throwable e) { + failure = e; + } + done.countDown(); + get(a); + pool.shutdown(); + t.line("A holds the lock; B asks with Timeouts.NO_WAIT"); + t.line("B exception: " + (failure == null ? "none" : describe(failure))); + t.blank().line("select statements B sent:"); + db.sql.stream().filter(x -> x.startsWith("select")).forEach(t::line); + assertThat(failure).isNotNull(); + } + } + + @Test + void skipLockedGivesEachWorkerDifferentRows() { + try (Db db = new Db(Ticket.class); Transcript t = new Transcript("06-skip-locked")) { + db.sessionFactory.inTransaction(s -> { + for (long i = 1; i <= 5; i++) { + s.persist(new Ticket(i, "NEW")); + } + }); + db.clear(); + CountDownLatch aHolds = new CountDownLatch(1); + CountDownLatch bDone = new CountDownLatch(1); + List claimedByA = new ArrayList<>(); + List claimedByB = new ArrayList<>(); + ExecutorService pool = Executors.newFixedThreadPool(2); + Future a = pool.submit(() -> db.sessionFactory.inTransaction(s -> { + claim(s, 2).forEach(tk -> claimedByA.add(tk.id)); + aHolds.countDown(); + await(bDone); + })); + await(aHolds); + db.sql.clear(); + db.sessionFactory.inTransaction(s -> claim(s, 2).forEach(tk -> claimedByB.add(tk.id))); + List bSql = new ArrayList<>(db.sql); + bDone.countDown(); + get(a); + pool.shutdown(); + t.line("5 tickets, all NEW. Worker A claims 2 and keeps its transaction open."); + t.line("Worker B claims 2 with SKIP LOCKED."); + t.line("A got tickets: " + claimedByA); + t.line("B got tickets: " + claimedByB); + t.blank().line("select statements B sent:"); + bSql.stream().filter(x -> x.startsWith("select")).forEach(t::line); + assertThat(claimedByA).containsExactly(1L, 2L); + assertThat(claimedByB).containsExactly(3L, 4L); + } + } + + private static List claim(org.hibernate.Session s, int n) { + List rows = s.createQuery("from Ticket t where t.status = 'NEW' order by t.id", Ticket.class) + .setMaxResults(n) + .setLockMode(LockModeType.PESSIMISTIC_WRITE) + .setHint("jakarta.persistence.lock.timeout", Timeouts.SKIP_LOCKED_MILLI) + .getResultList(); + rows.forEach(tk -> tk.status = "CLAIMED"); + return rows; + } + + @Test + void deadlockAndTheFix() { + try (Db db = new Db(Account.class); Transcript t = new Transcript("07-deadlock")) { + db.sessionFactory.inTransaction(s -> { + s.persist(new Account(1L, "alice", 100)); + s.persist(new Account(2L, "bob", 100)); + }); + + // opposite lock order: A takes 1 then 2, B takes 2 then 1 + List failures = transfer(db, true, 1L, 2L, 2L, 1L); + t.line("opposite lock order (A: 1 then 2, B: 2 then 1)"); + t.line("transactions that failed: " + failures.size()); + failures.forEach(f -> t.line(" " + describe(f))); + + // same order for everyone: lowest id first + List ordered = transfer(db, false, 1L, 2L, 1L, 2L); + t.blank().line("same lock order for both (always the lower id first)"); + t.line("transactions that failed: " + ordered.size()); + assertThat(ordered).isEmpty(); + assertThat(failures).isNotEmpty(); + } + } + + /** Two transactions; each locks its first id (optionally waiting until both hold one), then its second id. */ + private static List transfer(Db db, boolean syncAfterFirstLock, long aFirst, long aSecond, long bFirst, long bSecond) { + CountDownLatch bothHoldFirst = new CountDownLatch(2); + ExecutorService pool = Executors.newFixedThreadPool(2); + List> results = new ArrayList<>(); + long[][] plans = {{aFirst, aSecond}, {bFirst, bSecond}}; + for (long[] plan : plans) { + results.add(pool.submit(() -> { + try { + db.sessionFactory.inTransaction(s -> { + s.find(Account.class, plan[0], LockModeType.PESSIMISTIC_WRITE); + if (syncAfterFirstLock) { + bothHoldFirst.countDown(); + try { + bothHoldFirst.await(3, java.util.concurrent.TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + s.find(Account.class, plan[1], LockModeType.PESSIMISTIC_WRITE); + }); + return null; + } catch (Throwable e) { + return e; + } + })); + } + List failures = new ArrayList<>(); + for (Future f : results) { + try { + Throwable e = f.get(); + if (e != null) { + failures.add(e); + } + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + pool.shutdown(); + return failures; + } + + private static void sleep(long ms) { + try { + Thread.sleep(ms); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } +} diff --git a/locking/src/test/java/com/ankurm/locking/Transcript.java b/locking/src/test/java/com/ankurm/locking/Transcript.java new file mode 100644 index 0000000..20f383b --- /dev/null +++ b/locking/src/test/java/com/ankurm/locking/Transcript.java @@ -0,0 +1,52 @@ +package com.ankurm.locking; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +/** + * Collects lines and writes them to output/NN-name.txt when closed. The tests assert the same + * numbers they print, so every figure in the transcripts is an assertion that fails the build + * when it stops being true. + */ +final class Transcript implements AutoCloseable { + + private final Path file; + private final List lines = new ArrayList<>(); + + Transcript(String name) { + this.file = Path.of("output", name + ".txt"); + } + + Transcript line(String s) { + lines.add(s); + return this; + } + + Transcript blank() { + lines.add(""); + return this; + } + + /** Hibernate emits one long line per statement; break it at the keywords so a reader can follow the joins. */ + Transcript sql(String ignoredLabel, String sql) { + String pretty = sql + .replace(" union all select", "\nunion all\nselect") + .replaceAll(" (left join|inner join|cross join|join|where|group by|order by) ", "\n $1 ") + .replaceAll(" from ", "\nfrom "); + lines.add(pretty + ";"); + return this; + } + + @Override + public void close() { + try { + Files.createDirectories(file.getParent()); + Files.write(file, lines); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } +}