gnodet commented on PR #29564: URL: https://github.com/apache/camel/pull/29564#issuecomment-6820116103
Thank you both for the thorough review. You are right that the original remove-then-put approach (from the first commit) reintroduced the CAMEL-26395 crash window and incorrectly deleted the regression test. The fix was updated (the current HEAD `c50da046`) to the transactional approach you both suggested: - `tCache.remove(key, holder)` — conditional remove in the transaction; returns `false` for the loser, which rolls back and throws `OptimisticLockingException` without touching `persistedCache` - `tPersistentCache.put(id, holder)` — recovery write in the same transaction This closes both failure windows: 1. **Race (CAMEL-26796)**: the loser's transaction rolls back without touching the recovery map, so the winner's entry is never corrupted. 2. **Crash window (CAMEL-26395)**: a JVM stop mid-transaction rolls back; the group re-appears in the operational repository for duplicate delivery rather than silent loss. The non-recovery (`useRecovery=false`) path keeps the plain `cache.remove(key, holder)` without a transaction. The `checkOptimisticRemoveRaceDoesNotDeleteWinnersRecoveryEntry` regression test is kept. The two tests covering the old cleanup path are still removed since that path is gone. The 9 aggregation repository tests pass locally. @oscerd: on the crash-window claim from the first version — you were right to question it. With "remove-then-put", a crash between the two steps would be a loss, not a re-appearance. The description was wrong. With the transaction, the rollback on crash means the entry re-appears (safe side of the trade-off), so the description in the current PR body is accurate. _Note: this comment was generated by an AI coding agent on behalf of gnodet._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
