[#4453] Fix ReplayToken.wasProcessedBeforeReset() for tokens without gap-walkback lowerBound semantics#4458
Conversation
…back lowerBound semantics wasProcessedBeforeReset() relied solely on lowerBound() + samePositionAs() to detect whether an event was already processed before a reset. This works for GapAwareTrackingToken, whose lowerBound() walks back from gap positions, but breaks for token implementations whose lowerBound() computes a set-intersection (e.g. MongoTrackingToken). When the reset token and the replayed event token are from completely different time periods, their event-ID sets are disjoint, the intersection is empty, and samePositionAs(empty, other) returns false — causing every replayed event to be misclassified as "new" instead of "replay". This misclassification has two effects: 1. @DisallowReplay handlers fire and ReplayToken.isReplay() returns false during a replay. 2. advancedTo() falls into the "new event" branch and calls tokenAtReset.upperBound(newToken) for every replayed event, merging all event IDs into the stored token on every step and growing it without bound until the backing store's size limit is exceeded. The fix adds a covers() fallback: if resetLowerBound.covers(newTokenLowerBound) is true, the event was processed before the reset regardless of the lowerBound() result. For GapAwareTrackingToken the fallback is dead code — whenever covers() would return true, lowerBound() + samePositionAs() already does. For set-intersection tokens the fallback restores correct behaviour by using their time/position-based covers() implementation. Fixes: #4453
This allows testcontainers to run on environments with more recent Docker installations.
|
How (if so) will this affect the 5.1 branch? |
|
Did a local test with this fix. Replays are working again for our MongoDB event store, thanks! |
smcvb
left a comment
There was a problem hiding this comment.
Awesome job, looks good to me.
That feedback is extremely helpful, @martijnvanderwoud, thank you!! 🙏 |
@smcvb you are right, this reduces the impact of AxonFramework/extension-mongo#498 to almost zero. The unbounded growth now only occurs if the initial token at reset has an empty events collection, which is rare and easy to work around Thanks guys for fixing this so quickly! |
|
Sure thing @martijnvanderwoud! And thank you for filing this with us! |
It might, once we introduce tokens that are akin to the |
…non-gap-walkback-tokens
|
❌ The last analysis has failed. |
ReplayToken.wasProcessedBeforeReset()relied solely onlowerBound()+samePositionAs()to determine whether a replayed event had already been processed before the reset. This works correctly forGapAwareTrackingToken, whoselowerBound()walks back from gap positions, but breaks for token implementations whoselowerBound()computes a set-intersection — such asMongoTrackingToken.When the reset token and the replayed event token are from completely different time periods, their event-ID sets are disjoint. The intersection is empty, and
samePositionAs(empty, other)returnsfalse, causing every replayed event to be misclassified as "new" instead of "replay". This has two observable effects:@DisallowReplayhandlers fire during a replay, andReplayToken.isReplay()returnsfalse.advancedTo()falls into the "new event" branch and callstokenAtReset.upperBound(newToken)for every replayed event, merging all event IDs into the stored token on every step. The token grows without bound until the backing store's size limit is exceeded.The fix adds a
covers()fallback: ifresetLowerBound.covers(newTokenLowerBound)is true, the event was processed before the reset regardless of whatlowerBound()returned. ForGapAwareTrackingTokenthis fallback is dead code — whenevercovers()would returntrue, the existinglowerBound()+samePositionAs()check already does. For set-intersection tokens it restores correct behaviour by using their time/position-basedcovers()implementation.Two regression tests are added using a
WindowedTrackingToken(an in-test stub whoselowerBound()returns the intersection of event-ID sets, mirroringMongoTrackingToken):tokenAtResetdoes not grow during replayAlso includes an unrelated bump of the TestContainers version to support running integration tests against more recent Docker installations.
Fixes #4453.