Skip to content

fix: move the check for duplicate series up the call stack to fix an issue when retention is enabled - #17663

Merged
sandeepsukhani merged 1 commit into
mainfrom
deletion-series-progress-fix
May 12, 2025
Merged

fix: move the check for duplicate series up the call stack to fix an issue when retention is enabled#17663
sandeepsukhani merged 1 commit into
mainfrom
deletion-series-progress-fix

Conversation

@sandeepsukhani

@sandeepsukhani sandeepsukhani commented May 12, 2025

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
Deletion supports saving the progress of a series to be able to skip series which are already been processed. However, it requires all the chunks for a series to be sent at once to make it easier to save the progress.

Since both retention and deletion implement the same interfaces, the table iterator asks them to decide whether a series should be skipped. So, if retention is enabled and it says no to skipping the series while deletion has already processed it, we have to iterate through the chunks in that series again. While deletion still efficiently handles it, at the end, we again try to mark the series as processed, which then triggers the check for duplicate series and fails the operation.

This PR fixes the issue by moving the code to see a series only once in a table up the stack without persisting it. We just want a single series to be seen only once with all its chunks instead of a partial stream seen multiple times.

@sandeepsukhani
sandeepsukhani requested a review from a team as a code owner May 12, 2025 09:27
@sandeepsukhani
sandeepsukhani enabled auto-merge (squash) May 12, 2025 09:36

@paul1r paul1r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, we should probably add a test case for this longer term, for validation of this behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants