fix(process-engine): survive optimistic-lock races on task, process, and context rows#151
Merged
Merged
Conversation
…and context rows
Under load the aggregator's backup flow kept dying with
VersionMismatchException ('Version in repository: 2, Task Version 1' /
'Current version are less than saved') from BackupDatabaseTask: several
actors hold independent in-memory copies of the same row — the POProcess
tick (every 2s), the executing task, ProcessTaskFailureHandler, and the
TaskExecutorService timeout watchdog — and the loser of a
milliseconds-wide race aborted its completion, leaving the db-scheduler
execution stuck picked until dead-execution revival. Externally that
surfaced as hung backups and HTTP 500s.
Three fixes:
- All three repositories now guard the UPDATE itself (WHERE version=?)
instead of a check-then-act compare, closing the silent lost-update
window, and bump the in-memory version only on success.
- Must-win writes recover instead of aborting: terminal COMPLETED/FAILED
saves go through TaskInstanceImpl.saveResolvingConflict (reload,
re-apply, save), and DataContext.apply re-applies its idempotent
mutation on a fresh copy after a conflict.
- TaskExecutionWrapper ran its callback twice on failure (catch plus
finally); it now runs exactly once and logs the error.
…after recovery Review findings on the race fix: - Process-level terminal writes (POProcess completion FAILED/COMPLETED, AsyncTaskWithPolling failure, the timeout watchdog) still called raw save() and could abort on the same version race the task writes were protected from. ProcessInstanceImpl gains the same saveResolvingConflict, used at all four sites; the pre-existing reload() is reused. - After conflict recovery both saveResolvingConflict variants and DataContext.apply now sync the caller's instance with the persisted state (contents, fields, version, clean dirty flag). Without that the caller kept a stale copy and the very next save() — for example the trailing process save in the POProcess completion handler — failed again despite the recovery having succeeded.
- FutureKey.equals no longer accepts String/UUID (equals-contract violation); terminate() now filters by getTaskId() explicitly (S2159) - re-interrupt the thread on InterruptedException in the watchdog and in terminate()'s wait loop (S2142) - terminate() returns Future<Void> instead of Future<?> (S1452) - TaskExecutionWrapper catches Exception, not Throwable (S1181) - ProcessTaskFailureHandler reuses a single Optional instance (S3655) - suppress S3011 (reflective access into db-scheduler internals) and S2160 (HashMap content equality is intentional) with justifications
kichasov
force-pushed
the
fix/process-engine-version-races
branch
from
July 23, 2026 19:02
7a8da29 to
36b4a12
Compare
An interrupt is the watchdog's normal shutdown signal: the completion callback in execute() cancels it with cancel(true) once the task finishes in time. The shared catch treated that interrupt as a timeout and marked the completed task and its process FAILED — and saveResolvingConflict made the bogus overwrite reliable instead of losing the version race. Split the catch: InterruptedException restores the interrupt flag and stops watching without touching state; only ExecutionException and TimeoutException cancel the worker and mark the task and process FAILED. The watchdog body moved to a package-private watchTask() so tests can drive both paths directly.
|
alsergs
approved these changes
Jul 24, 2026
kichasov
added a commit
to Netcracker/qubership-dbaas
that referenced
this pull request
Jul 24, 2026
…ck race fixes (#603) Picks up Netcracker/qubership-core-java-libs#151, merged to main and published as 1.5.5-20260724.091358-10: atomic version guards on the task, process, and context rows, conflict-recovering terminal saves, a single completion callback per task, and a watchdog that no longer fails a task that completed in time. AbstractDbaaSTask.updateState switches from put+save to context.apply so its status-description writes get the same conflict recovery; a plain save() aborted the whole task when the POProcess tick or the timeout watchdog wrote the same row first. Verified on verify/process-engine-race-fix and verify/pe-race-fix-nb-status against a throwaway snapshot of the same code: the DBaaS IT concurrent section passes and the VersionMismatchException signature is gone from the aggregator logs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The dbaas-aggregator's backup flow keeps failing under load with
VersionMismatchExceptionfromBackupDatabaseTask(four DBaaS IT runs bitten so far; diagnostics in Netcracker/qubership-dbaas runs 29988579829 and 30026600214):Several actors hold independent in-memory copies of the same
pe_task_instance/po_context/ process rows — the POProcess tick (every 2 s), the executing task,ProcessTaskFailureHandler, and theTaskExecutorServicetimeout watchdog. Whoever loses a milliseconds-wide race aborts its completion; the db-scheduler execution then stays picked until dead-execution revival, and externally that is a hung backup and an HTTP 500.Two defects compound it: the repositories used a check-then-act version compare (SELECT, compare in Java, unguarded UPDATE), which under true concurrency also silently loses one of two writes; and
TaskExecutionWrapperran its completion callback twice on failure.What
WHERE ... AND version=?, affected-row check) and bump the in-memory version only after success. Exception contract unchanged.COMPLETED/FAILEDsaves go through newTaskInstanceImpl.saveResolvingConflict(reapply)— on a conflict the row is reloaded, the idempotent mutation re-applied, and the fresh copy saved (used inAbstractProcessTask,ProcessTaskFailureHandler,AsyncTaskWithPolling, the timeout watchdog).DataContext.applydoes the same for context mutations, which covers task start/end times and state descriptions.TaskExecutionWrapperruns its callback exactly once and logs the swallowed error.A completion that today crashes with a stuck execution would, with slightly different timing, have succeeded anyway — the recovery path persists exactly that outcome, so no new interleavings are introduced.
How to verify
mvn -pl core-process-orchestrator test— 37 tests green, including the newVersionConflictTest: stale-copy failures for all three repositories (winning write survives, no corruption), terminal-save recovery via reload, contextapplyrecovery preserving the concurrent writer's data, and single-callback wrapper behavior.After merge, the consumer side (Netcracker/qubership-dbaas) needs a
process-enginebump (currently pinned to 1.5.2) plus switchingAbstractDbaaSTask.updateStatetocontext.apply(...)so its status-description writes get the same recovery — I will follow up there and re-run the DBaaS IT that reproduces the race.