Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filter out blocks marked for deletion in the bucket index ids fetcher… #5712

Conversation

wenxu1024
Copy link
Contributor

@wenxu1024 wenxu1024 commented Dec 16, 2023

filter out blocks marked for deletion in the bucket index ids fetcher, so we are protected even when apply ignoreDeletionMarkerFilter the block has been deleted.

What this PR does:

Previously in this pr #5681
we introduced bucketindex BlockIDsFetcher.

In that IDs fetcher, we just send all blocks to the chan, and didn't filter out the blocks that are marked for deletion.

During compaction run, when we try to apply the ignoreDeletionMarkerFilter, because some blocks marked for deletion are already deleted by the cleaner. Then the we failed to filter out those blocks.

Then during compaction, when we try to download the meta.json from those blocks, we get an error, because there is no meta.json in the block. The block is already deleted.

This PR try to fix the problem by filtering out the blocks marked for deletion in the bucket-index ids fetcher. Then apply the ignoreDeletionMarkerFilter again outside.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

… too, so we are protected even when apply ignoreDeletionMarkerFilter the block has been deleted

Signed-off-by: Wen Xu <[email protected]>
@wenxu1024 wenxu1024 marked this pull request as ready for review December 18, 2023 16:22
@alexqyle
Copy link
Contributor

LGTM

@yeya24 yeya24 merged commit aef8ad3 into cortexproject:master Dec 19, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants