ReentrantLock cannot prevent concurrent updates if multi-process is using #5020

Activity

1. quaff

   added

   status: waiting-for-triageIssues that we did not analyse yetIssues that we did not analyse yet

   type: bug

   on Oct 11, 2025
2. n-dlms commented on Aug 4, 2026

   

   n-dlms

   on Aug 4, 2026

   More actions

   I analyzed this issue and gathered some facts that might help with the triage.

   Where the locks are

   All three JDBC DAOs hold a single private final Lock lock = new ReentrantLock() per DAO instance, guarding the update methods:

   • JdbcStepExecutionDao#updateStepExecution
   • JdbcJobExecutionDao#updateJobExecution
   • JdbcExecutionContextDao#updateExecutionContext

   History

   The serialization is not new: it goes back to synchronized blocks in the original DAO code, which were replaced with ReentrantLock in #5bccfed5 (2023-07-15, "Replace synchronized blocks and methods with locks"). Since June 2025 (#4792, #4793), the update SQL is version-predicated (SET VERSION = VERSION + 1 ... WHERE ... AND VERSION = ?) and throws OptimisticLockingFailureException when count == 0, so the optimistic check is now the real safety net and the lock is largely redundant.

   What the lock actually protects

   It serializes, within one JVM, the sequence: read the in-memory version -> build the UPDATE -> execute it -> stepExecution.incrementVersion(). Relevant same-row concurrent writers within a JVM:

   1. Multithreaded/async chunk steps, where several worker threads share one StepExecution instance and update it concurrently from chunk commit callbacks.
   2. The stop/graceful-shutdown path (see GracefulShutdownFunctionalTests.testStopJob fails intermittently due to a race condition #5308 family): the operator thread and the step thread update the same execution.
   3. The restart path (Possible race condition on job restart #5397 family).

   It does not protect anything across JVMs (remote partitioning, clustered deployments) - there the optimistic check is the only guard.

   Impact of removing the locks

   For case (1), two threads can read the same in-memory version before either commits, so both build UPDATE ... WHERE VERSION = n; one succeeds and the other gets count == 0 -> OptimisticLockingFailureException, which today propagates out of jobRepository.update(stepExecution) and fails the chunk/step. In other words, removing the lock regresses multithreaded steps to the exception-based behavior; it does not make them fail silently. This deserves a regression test if we go ahead (there is currently no test covering concurrent updates of the same execution).

   For case (2), #5442/#5448 already remove the redundant double-write on the stop path at the source, which reduces the remaining same-row contention mostly to multithreaded steps.

   My take

   I agree the DAO-wide locks are the wrong mechanism: they are coarse (they serialize updates of unrelated executions too - e.g. parallel steps or partitioned workers in one JVM), JVM-local (so they give a false sense of protection in clustered setups), and they hold a lock around blocking I/O.

   I would be happy to prepare a PR that removes the locks from the three DAOs and relies on the optimistic check, along with a test demonstrating concurrent same-row updates now surface as OptimisticLockingFailureException (so the behavior change is explicit and documented). Should I proceed?
3. quaff commented on Aug 5, 2026

   

   quaff

   on Aug 5, 2026

   ContributorAuthor

   More actions

   they are coarse (they serialize updates of unrelated executions too - e.g. parallel steps or partitioned workers in one JVM), JVM-local (so they give a false sense of protection in clustered setups), and they hold a lock around blocking I/O.

   See #5095