Fix PurgeEx replay with sequence & keep succeeds #4213
Merged
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.
PR #4212 fixed the issue I reported in #4196.
However, I believe there might be a bug when both
sequence
andkeep
are set during recovery.In the
PurgeEx
the following check is done (for bothfilestore.go
andmemstore.go
):The
TestJetStreamClusterPurgeExReplayAfterRestart
also triggers this case, meaning that during the test this error is returned but it succeeds because the purge was already performed. Is this intended behaviour?To elaborate a bit more, I believe the following happens:
keep
(since it's not combined withsequence
yet)sequence
is added to thekeep
, which errors out in the above ifWhich means that during normal operation all will be well, but purges with
keep
will be ignored upon replaying.I'm proposing to remove the
sequence > 1 && keep > 0
check and subsequent error. Which, for reference, was introduced in #3121.Hoping this ensures that during recovery, purges that haven't executed yet will still be executed.
An alternative approach, which wouldn't remove the error: not allow combining
sequence
andkeep
normally and only allowing it during recovery. Which would preserve the current behaviour, and correctly applysequence+keep
during recovery still. However, not sure if it's possible to know if we're in "recovery mode" from withinPurgeEx
.Resolves #4196