Skip to content

Commit

Permalink
Fix a bug in ReFitLevel() where FileMetaData::being_compacted is no…
Browse files Browse the repository at this point in the history
…t 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: #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
  • Loading branch information
cbi42 authored and facebook-github-bot committed Sep 12, 2024
1 parent 43bc71f commit e490f2b
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 12 deletions.
9 changes: 4 additions & 5 deletions db/compaction/compaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions db/compaction/compaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<CompactionInputFiles>& inputs,
Expand Down
3 changes: 2 additions & 1 deletion db/compaction/compaction_picker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion db/compaction/compaction_picker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<FileMetaData*>& files);
Expand Down
2 changes: 1 addition & 1 deletion db/db_impl/db_impl_compaction_flush.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
1 change: 1 addition & 0 deletions unreleased_history/bug_fixes/bug-refit-level.md
Original file line number Diff line number Diff line change
@@ -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).

0 comments on commit e490f2b

Please sign in to comment.