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

db: use objstorage to find obsolete objects #2347

Merged
merged 1 commit into from
Feb 21, 2023

Conversation

RaduBerinde
Copy link
Member

Ignore SST files in the listing and instead get the list of objects from the objstorage provider.

@RaduBerinde RaduBerinde requested review from bananabrick and a team February 17, 2023 20:29
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@bananabrick bananabrick left a comment

Choose a reason for hiding this comment

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

Code looks :lgtm:

Could we test this, at least manually.

  1. Create some fake sst files using the sst file naming format in the Pebble directory.
  2. Reopen pebble. Pebble won't try to read the fake sst files and in the scanObsoleteFiles step, they should be added to the obsolete file list.
  3. Make sure that the fake sst files were deleted by Pebble.

Reviewable status: 0 of 3 files reviewed, all discussions resolved

Copy link
Member Author

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

Good idea, I extended the cleaner test.

Reviewable status: 0 of 5 files reviewed, all discussions resolved

Ignore SST files in the listing and instead get the list of objects
from the objstorage provider.

Extend the cleaner test to check for removal of extra sst files.
@RaduBerinde RaduBerinde merged commit efd802f into cockroachdb:master Feb 21, 2023
@RaduBerinde RaduBerinde deleted the list-objects branch February 21, 2023 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants