fix(ledger): move optimistic retry outside the idempotency transaction - #136
Merged
Conversation
added 2 commits
June 15, 2026 14:40
The optimistic-retry loop lived in TransactionServiceImpl, between the @transactional IdempotencyStore.runOrReplay that opens the unit-of-work transaction and the @transactional TransactionPoster that joins it. On an OptimisticLockingFailureException the joined poster marked the shared transaction rollback-only, so the retry re-entered a doomed transaction and the commit raised UnexpectedRollbackException. Retry never worked on the real request path; it only passed where the poster was mocked. Move the bounded retry loop into IdempotencyServiceImpl.execute so each attempt re-invokes runOrReplay in a fresh physical transaction. Reservation, business rows and audit row commit or roll back together per attempt, with no REQUIRES_NEW and no split reservation. The idempotency race recovery stays single-shot and does not consume a retry attempt; a body conflict still propagates without retry. Add AuditRetryTopologyIT contended cases on real committing boundaries with a forced optimistic failure, asserting exactly one business and audit row on retry and zero of each plus no reserved key on full rollback. Closes #134
… tests AuditRetryTopologyIT runs with NOT_SUPPORTED so each runOrReplay commits for real (required to exercise the retry-in-a-fresh-transaction topology). The committed outbox rows leaked into the shared Testcontainers Postgres and inflated the global-count outbox assertions in TransactionBalanceServiceIT and OutboxEventPublisherIT. Delete the outbox table in @AfterEach, mirroring OutboxEventPublisherIT. Audit rows stay (append-only trigger 018 rejects delete); other ITs scope audit by unique resourceId so they are unaffected.
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
1 participant
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.
What
The optimistic-retry loop sat in
TransactionServiceImpl, between the@TransactionalIdempotencyStore.runOrReplay(which opens the unit-of-work transaction TX#1) and the@TransactionalTransactionPoster(which joins TX#1). On anOptimisticLockingFailureExceptionthe joined poster marked the shared TX#1 rollback-only, so the retry re-entered a doomed transaction and the commit raisedUnexpectedRollbackException. The retry never worked on the real request path; it only passed where the poster was a MockK mock.This moves the bounded retry loop into
IdempotencyServiceImpl.execute, so each attempt re-invokesrunOrReplayin a fresh physical transaction. The idempotency reservation, business rows and audit row commit or roll back together per attempt (noREQUIRES_NEW, no split reservation). The race recovery stays single-shot and does not consume a retry attempt; a body conflict still propagates without retry.Why
Pre-existing correctness defect surfaced by the #49 pre-mortem. Under real contention on a hot account, the broken retry could lose a committed-looking write together with its audit row - an audit-integrity gap on exactly the operations auditors scrutinise.
Changes
IdempotencyServiceImpl: bounded optimistic-retry loop (MAX_ATTEMPTS=3) wrappingstore.runOrReplay;ConcurrencyConflictExceptionon exhaustion (503 + Retry-After, unchanged surface).TransactionServiceImpl:post/reversebecome plain delegations;withOptimisticRetryandMAX_ATTEMPTSremoved.IdempotencyServiceImplTest;AuditRetryTopologyITgains contended cases on real committing boundaries (NOT_SUPPORTED) with a forced optimistic failure - exactly one business + audit row on retry, zero of each plus no reserved key on full rollback.Verification
Local gates green:
:test(11/11 unit),:detekt,:spotlessCheck,:assemble,:compileIntegrationTestKotlin. The contended integration tests run on CI (Testcontainers).Closes #134