Skip to content

Commit

Permalink
Fix blob files not reclaimed after deleting all SSTs (facebook#12235)
Browse files Browse the repository at this point in the history
Summary:
Fix issue facebook#12208.

After all the SSTs have been deleted, all the blob files will become unreferenced.
These files should be considered obsolete and thus, should not be saved to the vstorage.

Pull Request resolved: facebook#12235

Reviewed By: jowlyzhang

Differential Revision: D52806441

Pulled By: ltamasi

fbshipit-source-id: 62f94d4f2544ed2822c764d8ace5bf7f57efe42d
  • Loading branch information
jonahgao authored and facebook-github-bot committed Jan 16, 2024
1 parent 2dda7a0 commit e28251c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 6 deletions.
6 changes: 6 additions & 0 deletions db/version_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,12 @@ class VersionBuilder::Rep {
const uint64_t oldest_blob_file_with_linked_ssts =
GetMinOldestBlobFileNumber();

// If there are no blob files with linked SSTs, meaning that there are no
// valid blob files
if (oldest_blob_file_with_linked_ssts == kInvalidBlobFileNumber) {
return;
}

auto process_base =
[vstorage](const std::shared_ptr<BlobFileMetaData>& base_meta) {
assert(base_meta);
Expand Down
35 changes: 29 additions & 6 deletions db/version_builder_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1194,24 +1194,47 @@ TEST_F(VersionBuilderTest, SaveBlobFilesTo) {

ASSERT_OK(second_builder.Apply(&second_edit));

VersionStorageInfo newer_vstorage(
VersionStorageInfo new_vstorage_2(
&icmp_, ucmp_, options_.num_levels, kCompactionStyleLevel, &new_vstorage,
force_consistency_checks, EpochNumberRequirement::kMightMissing, nullptr,
0, OffpeakTimeOption(options_.daily_offpeak_time_utc));

ASSERT_OK(second_builder.SaveTo(&newer_vstorage));
ASSERT_OK(second_builder.SaveTo(&new_vstorage_2));

UpdateVersionStorageInfo(&newer_vstorage);
UpdateVersionStorageInfo(&new_vstorage_2);

const auto& newer_blob_files = newer_vstorage.GetBlobFiles();
const auto& newer_blob_files = new_vstorage_2.GetBlobFiles();
ASSERT_EQ(newer_blob_files.size(), 2);

const auto newer_meta3 =
newer_vstorage.GetBlobFileMetaData(/* blob_file_number */ 3);
new_vstorage_2.GetBlobFileMetaData(/* blob_file_number */ 3);

ASSERT_EQ(newer_meta3, nullptr);

UnrefFilesInVersion(&newer_vstorage);
// Blob file #5 is referenced by table file #4, and blob file #9 is
// unreferenced. After deleting table file #4, all blob files will become
// unreferenced and will therefore be obsolete.
VersionBuilder third_builder(env_options, &ioptions_, table_cache,
&new_vstorage_2, version_set);
VersionEdit third_edit;
third_edit.DeleteFile(/* level */ 0, /* file_number */ 4);

ASSERT_OK(third_builder.Apply(&third_edit));

VersionStorageInfo new_vstorage_3(
&icmp_, ucmp_, options_.num_levels, kCompactionStyleLevel,
&new_vstorage_2, force_consistency_checks,
EpochNumberRequirement::kMightMissing, nullptr, 0,
OffpeakTimeOption(options_.daily_offpeak_time_utc));

ASSERT_OK(third_builder.SaveTo(&new_vstorage_3));

UpdateVersionStorageInfo(&new_vstorage_3);

ASSERT_TRUE(new_vstorage_3.GetBlobFiles().empty());

UnrefFilesInVersion(&new_vstorage_3);
UnrefFilesInVersion(&new_vstorage_2);
UnrefFilesInVersion(&new_vstorage);
}

Expand Down

0 comments on commit e28251c

Please sign in to comment.