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
This commit is contained in:
Changyu Bi 2024-09-12 15:19:14 -07:00 committed by Facebook GitHub Bot
parent 43bc71fef6
commit e490f2b051
6 changed files with 13 additions and 12 deletions

View File

@ -686,12 +686,11 @@ bool Compaction::KeyRangeNotExistsBeyondOutputLevel(
}; };
// Mark (or clear) each file that is being compacted // 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 i = 0; i < num_input_levels(); i++) {
for (size_t j = 0; j < inputs_[i].size(); j++) { for (size_t j = 0; j < inputs_[i].size(); j++) {
assert(mark_as_compacted ? !inputs_[i][j]->being_compacted assert(being_compacted != inputs_[i][j]->being_compacted);
: inputs_[i][j]->being_compacted); inputs_[i][j]->being_compacted = being_compacted;
inputs_[i][j]->being_compacted = mark_as_compacted;
} }
} }
} }
@ -735,7 +734,7 @@ uint64_t Compaction::CalculateTotalInputSize() const {
return size; return size;
} }
void Compaction::ReleaseCompactionFiles(Status status) { void Compaction::ReleaseCompactionFiles(const Status& status) {
MarkFilesBeingCompacted(false); MarkFilesBeingCompacted(false);
cfd_->compaction_picker()->ReleaseCompactionFiles(this, status); cfd_->compaction_picker()->ReleaseCompactionFiles(this, status);
} }

View File

@ -230,7 +230,7 @@ class Compaction {
// Delete this compaction from the list of running compactions. // Delete this compaction from the list of running compactions.
// //
// Requirement: DB mutex held // Requirement: DB mutex held
void ReleaseCompactionFiles(Status status); void ReleaseCompactionFiles(const Status& status);
// Returns the summary of the compaction in "output" with maximum "len" // Returns the summary of the compaction in "output" with maximum "len"
// in bytes. The caller is responsible for the memory management of // in bytes. The caller is responsible for the memory management of
@ -435,13 +435,13 @@ class Compaction {
const int start_level, const int start_level,
const int output_level); const int output_level);
// mark (or clear) all files that are being compacted
void MarkFilesBeingCompacted(bool being_compacted) const;
private: private:
Status InitInputTableProperties(); 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 // get the smallest and largest key present in files to be compacted
static void GetBoundaryKeys(VersionStorageInfo* vstorage, static void GetBoundaryKeys(VersionStorageInfo* vstorage,
const std::vector<CompactionInputFiles>& inputs, const std::vector<CompactionInputFiles>& inputs,

View File

@ -133,7 +133,8 @@ CompactionPicker::CompactionPicker(const ImmutableOptions& ioptions,
CompactionPicker::~CompactionPicker() = default; CompactionPicker::~CompactionPicker() = default;
// Delete this compaction from the list of running compactions. // 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); UnregisterCompaction(c);
if (!status.ok()) { if (!status.ok()) {
c->ResetNextCompactionIndex(); c->ResetNextCompactionIndex();

View File

@ -104,7 +104,7 @@ class CompactionPicker {
// Free up the files that participated in a compaction // Free up the files that participated in a compaction
// //
// Requirement: DB mutex held // 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 // Returns true if any one of the specified files are being compacted
bool AreFilesInCompaction(const std::vector<FileMetaData*>& files); bool AreFilesInCompaction(const std::vector<FileMetaData*>& files);

View File

@ -1880,7 +1880,7 @@ Status DBImpl::ReFitLevel(ColumnFamilyData* cfd, int level, int target_level) {
Status status = versions_->LogAndApply(cfd, mutable_cf_options, Status status = versions_->LogAndApply(cfd, mutable_cf_options,
read_options, write_options, &edit, read_options, write_options, &edit,
&mutex_, directories_.GetDbDir()); &mutex_, directories_.GetDbDir());
c->MarkFilesBeingCompacted(false);
cfd->compaction_picker()->UnregisterCompaction(c.get()); cfd->compaction_picker()->UnregisterCompaction(c.get());
c.reset(); c.reset();

View File

@ -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).