From e28251ca729ed42a5a8d7181b703b2e059506573 Mon Sep 17 00:00:00 2001 From: Jonah Gao Date: Tue, 16 Jan 2024 11:15:23 -0800 Subject: [PATCH] Fix blob files not reclaimed after deleting all SSTs (#12235) Summary: Fix issue https://github.com/facebook/rocksdb/issues/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: https://github.com/facebook/rocksdb/pull/12235 Reviewed By: jowlyzhang Differential Revision: D52806441 Pulled By: ltamasi fbshipit-source-id: 62f94d4f2544ed2822c764d8ace5bf7f57efe42d --- db/version_builder.cc | 6 ++++++ db/version_builder_test.cc | 35 +++++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 210b0de869..9a72307d37 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -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& base_meta) { assert(base_meta); diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 2ca10c449c..3c7d8a61d7 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -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); }