From e490f2b051b53c019c7f318771cedffd5cf708b4 Mon Sep 17 00:00:00 2001 From: Changyu Bi Date: Thu, 12 Sep 2024 15:19:14 -0700 Subject: [PATCH] Fix a bug in ReFitLevel() where `FileMetaData::being_compacted` is not cleared (#13009) Summary: in ReFitLevel(), we were not setting being_compacted to false after ReFitLevel() is done. This is not a issue if refit level is successful, since new FileMetaData is created for files at the target level. However, if there's an error during RefitLevel(), e.g., Manifest write failure, we should clear the being_compacted field for these files. Otherwise, these files will not be picked for compaction until db reopen. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13009 Test Plan: existing test. - stress test failure in T200339331 should not happen anymore. Reviewed By: hx235 Differential Revision: D62597169 Pulled By: cbi42 fbshipit-source-id: 0ba659806da6d6d4b42384fc95268b2d7bad720e --- db/compaction/compaction.cc | 9 ++++----- db/compaction/compaction.h | 8 ++++---- db/compaction/compaction_picker.cc | 3 ++- db/compaction/compaction_picker.h | 2 +- db/db_impl/db_impl_compaction_flush.cc | 2 +- unreleased_history/bug_fixes/bug-refit-level.md | 1 + 6 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 unreleased_history/bug_fixes/bug-refit-level.md diff --git a/db/compaction/compaction.cc b/db/compaction/compaction.cc index 1c014663cb..fc76f93f1c 100644 --- a/db/compaction/compaction.cc +++ b/db/compaction/compaction.cc @@ -686,12 +686,11 @@ bool Compaction::KeyRangeNotExistsBeyondOutputLevel( }; // Mark (or clear) each file that is being compacted -void Compaction::MarkFilesBeingCompacted(bool mark_as_compacted) { +void Compaction::MarkFilesBeingCompacted(bool being_compacted) const { for (size_t i = 0; i < num_input_levels(); i++) { for (size_t j = 0; j < inputs_[i].size(); j++) { - assert(mark_as_compacted ? !inputs_[i][j]->being_compacted - : inputs_[i][j]->being_compacted); - inputs_[i][j]->being_compacted = mark_as_compacted; + assert(being_compacted != inputs_[i][j]->being_compacted); + inputs_[i][j]->being_compacted = being_compacted; } } } @@ -735,7 +734,7 @@ uint64_t Compaction::CalculateTotalInputSize() const { return size; } -void Compaction::ReleaseCompactionFiles(Status status) { +void Compaction::ReleaseCompactionFiles(const Status& status) { MarkFilesBeingCompacted(false); cfd_->compaction_picker()->ReleaseCompactionFiles(this, status); } diff --git a/db/compaction/compaction.h b/db/compaction/compaction.h index 22157eb2c3..633e68a9ea 100644 --- a/db/compaction/compaction.h +++ b/db/compaction/compaction.h @@ -230,7 +230,7 @@ class Compaction { // Delete this compaction from the list of running compactions. // // Requirement: DB mutex held - void ReleaseCompactionFiles(Status status); + void ReleaseCompactionFiles(const Status& status); // Returns the summary of the compaction in "output" with maximum "len" // in bytes. The caller is responsible for the memory management of @@ -435,13 +435,13 @@ class Compaction { const int start_level, const int output_level); + // mark (or clear) all files that are being compacted + void MarkFilesBeingCompacted(bool being_compacted) const; + private: Status InitInputTableProperties(); - // mark (or clear) all files that are being compacted - void MarkFilesBeingCompacted(bool mark_as_compacted); - // get the smallest and largest key present in files to be compacted static void GetBoundaryKeys(VersionStorageInfo* vstorage, const std::vector& inputs, diff --git a/db/compaction/compaction_picker.cc b/db/compaction/compaction_picker.cc index e049d95b24..cc47060b5f 100644 --- a/db/compaction/compaction_picker.cc +++ b/db/compaction/compaction_picker.cc @@ -133,7 +133,8 @@ CompactionPicker::CompactionPicker(const ImmutableOptions& ioptions, CompactionPicker::~CompactionPicker() = default; // Delete this compaction from the list of running compactions. -void CompactionPicker::ReleaseCompactionFiles(Compaction* c, Status status) { +void CompactionPicker::ReleaseCompactionFiles(Compaction* c, + const Status& status) { UnregisterCompaction(c); if (!status.ok()) { c->ResetNextCompactionIndex(); diff --git a/db/compaction/compaction_picker.h b/db/compaction/compaction_picker.h index 88915d4594..087595a8a6 100644 --- a/db/compaction/compaction_picker.h +++ b/db/compaction/compaction_picker.h @@ -104,7 +104,7 @@ class CompactionPicker { // Free up the files that participated in a compaction // // Requirement: DB mutex held - void ReleaseCompactionFiles(Compaction* c, Status status); + void ReleaseCompactionFiles(Compaction* c, const Status& status); // Returns true if any one of the specified files are being compacted bool AreFilesInCompaction(const std::vector& files); diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index 14dd02b57f..3fb8af4477 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -1880,7 +1880,7 @@ Status DBImpl::ReFitLevel(ColumnFamilyData* cfd, int level, int target_level) { Status status = versions_->LogAndApply(cfd, mutable_cf_options, read_options, write_options, &edit, &mutex_, directories_.GetDbDir()); - + c->MarkFilesBeingCompacted(false); cfd->compaction_picker()->UnregisterCompaction(c.get()); c.reset(); diff --git a/unreleased_history/bug_fixes/bug-refit-level.md b/unreleased_history/bug_fixes/bug-refit-level.md new file mode 100644 index 0000000000..f41e699e6a --- /dev/null +++ b/unreleased_history/bug_fixes/bug-refit-level.md @@ -0,0 +1 @@ +* Fix a bug in CompactRange() where result files may not be compacted in any future compaction. This can only happen when users configure CompactRangeOptions::change_level to true and the change level step of manual compaction fails (#13009). \ No newline at end of file