Skip to content

Commit 98d5db5

Browse files
hx235facebook-github-bot
authored andcommitted
Sort L0 files by newly introduced epoch_num (#10922)
Summary: **Context:** Sorting L0 files by `largest_seqno` has at least two inconvenience: - File ingestion and compaction involving ingested files can create files of overlapping seqno range with the existing files. `force_consistency_check=true` will catch such overlap seqno range even those harmless overlap. - For example, consider the following sequence of events ("key@n" indicates key at seqno "n") - insert k1@1 to memtable m1 - ingest file s1 with k2@2, ingest file s2 with k3@3 - insert k4@4 to m1 - compact files s1, s2 and result in new file s3 of seqno range [2, 3] - flush m1 and result in new file s4 of seqno range [1, 4]. And `force_consistency_check=true` will think s4 and s3 has file reordering corruption that might cause retuning an old value of k1 - However such caught corruption is a false positive since s1, s2 will not have overlapped keys with k1 or whatever inserted into m1 before ingest file s1 by the requirement of file ingestion (otherwise the m1 will be flushed first before any of the file ingestion completes). Therefore there in fact isn't any file reordering corruption. - Single delete can decrease a file's largest seqno and ordering by `largest_seqno` can introduce a wrong ordering hence file reordering corruption - For example, consider the following sequence of events ("key@n" indicates key at seqno "n", Credit to ajkr for this example) - an existing SST s1 contains only k1@1 - insert k1@2 to memtable m1 - ingest file s2 with k3@3, ingest file s3 with k4@4 - insert single delete k5@5 in m1 - flush m1 and result in new file s4 of seqno range [2, 5] - compact s1, s2, s3 and result in new file s5 of seqno range [1, 4] - compact s4 and result in new file s6 of seqno range [2] due to single delete - By the last step, we have file ordering by largest seqno (">" means "newer") : s5 > s6 while s6 contains a newer version of the k1's value (i.e, k1@2) than s5, which is a real reordering corruption. While this can be caught by `force_consistency_check=true`, there isn't a good way to prevent this from happening if ordering by `largest_seqno` Therefore, we are redesigning the sorting criteria of L0 files and avoid above inconvenience. Credit to ajkr , we now introduce `epoch_num` which describes the order of a file being flushed or ingested/imported (compaction output file will has the minimum `epoch_num` among input files'). This will avoid the above inconvenience in the following ways: - In the first case above, there will no longer be overlap seqno range check in `force_consistency_check=true` but `epoch_number` ordering check. This will result in file ordering s1 < s2 < s4 (pre-compaction) and s3 < s4 (post-compaction) which won't trigger false positive corruption. See test class `DBCompactionTestL0FilesMisorderCorruption*` for more. - In the second case above, this will result in file ordering s1 < s2 < s3 < s4 (pre-compacting s1, s2, s3), s5 < s4 (post-compacting s1, s2, s3), s5 < s6 (post-compacting s4), which are correct file ordering without causing any corruption. **Summary:** - Introduce `epoch_number` stored per `ColumnFamilyData` and sort CF's L0 files by their assigned `epoch_number` instead of `largest_seqno`. - `epoch_number` is increased and assigned upon `VersionEdit::AddFile()` for flush (or similarly for WriteLevel0TableForRecovery) and file ingestion (except for allow_behind_true, which will always get assigned as the `kReservedEpochNumberForFileIngestedBehind`) - Compaction output file is assigned with the minimum `epoch_number` among input files' - Refit level: reuse refitted file's epoch_number - Other paths needing `epoch_number` treatment: - Import column families: reuse file's epoch_number if exists. If not, assign one based on `NewestFirstBySeqNo` - Repair: reuse file's epoch_number if exists. If not, assign one based on `NewestFirstBySeqNo`. - Assigning new epoch_number to a file and adding this file to LSM tree should be atomic. This is guaranteed by us assigning epoch_number right upon `VersionEdit::AddFile()` where this version edit will be apply to LSM tree shape right after by holding the db mutex (e.g, flush, file ingestion, import column family) or by there is only 1 ongoing edit per CF (e.g, WriteLevel0TableForRecovery, Repair). - Assigning the minimum input epoch number to compaction output file won't misorder L0 files (even through later `Refit(target_level=0)`). It's due to for every key "k" in the input range, a legit compaction will cover a continuous epoch number range of that key. As long as we assign the key "k" the minimum input epoch number, it won't become newer or older than the versions of this key that aren't included in this compaction hence no misorder. - Persist `epoch_number` of each file in manifest and recover `epoch_number` on db recovery - Backward compatibility with old db without `epoch_number` support is guaranteed by assigning `epoch_number` to recovered files by `NewestFirstBySeqno` order. See `VersionStorageInfo::RecoverEpochNumbers()` for more - Forward compatibility with manifest is guaranteed by flexibility of `NewFileCustomTag` - Replace `force_consistent_check` on L0 with `epoch_number` and remove false positive check like case 1 with `largest_seqno` above - Due to backward compatibility issue, we might encounter files with missing epoch number at the beginning of db recovery. We will still use old L0 sorting mechanism (`NewestFirstBySeqno`) to check/sort them till we infer their epoch number. See usages of `EpochNumberRequirement`. - Remove fix #5958 (comment) and their outdated tests to file reordering corruption because such fix can be replaced by this PR. - Misc: - update existing tests with `epoch_number` so make check will pass - update #5958 (comment) tests to verify corruption is fixed using `epoch_number` and cover universal/fifo compaction/CompactRange/CompactFile cases - assert db_mutex is held for a few places before calling ColumnFamilyData::NewEpochNumber() Pull Request resolved: #10922 Test Plan: - `make check` - New unit tests under `db/db_compaction_test.cc`, `db/db_test2.cc`, `db/version_builder_test.cc`, `db/repair_test.cc` - Updated tests (i.e, `DBCompactionTestL0FilesMisorderCorruption*`) under #5958 (comment) - [Ongoing] Compatibility test: manually run ajkr@36a5686 (with file ingestion off for running the `.orig` binary to prevent this bug affecting upgrade/downgrade formality checking) for 1 hour on `simple black/white box`, `cf_consistency/txn/enable_ts with whitebox + test_best_efforts_recovery with blackbox` - [Ongoing] normal db stress test - [Ongoing] db stress test with aggressive value #10761 Reviewed By: ajkr Differential Revision: D41063187 Pulled By: hx235 fbshipit-source-id: 826cb23455de7beaabe2d16c57682a82733a32a9
1 parent 9b34c09 commit 98d5db5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+1780
-486
lines changed

HISTORY.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22
## Unreleased
33
### Behavior changes
44
* Make best-efforts recovery verify SST unique ID before Version construction (#10962)
5+
* Introduce `epoch_number` and sort L0 files by `epoch_number` instead of `largest_seqno`. `epoch_number` represents the order of a file being flushed or ingested/imported. Compaction output file will be assigned with the minimum `epoch_number` among input files'. For L0, larger `epoch_number` indicates newer L0 file.
56

67
### Bug Fixes
78
* Fixed a regression in iterator where range tombstones after `iterate_upper_bound` is processed.
89
* Fixed a memory leak in MultiGet with async_io read option, caused by IO errors during table file open
910
* Fixed a bug that multi-level FIFO compaction deletes one file in non-L0 even when `CompactionOptionsFIFO::max_table_files_size` is no exceeded since #10348 or 7.8.0.
1011
* Fixed a bug caused by `DB::SyncWAL()` affecting `track_and_verify_wals_in_manifest`. Without the fix, application may see "open error: Corruption: Missing WAL with log number" while trying to open the db. The corruption is a false alarm but prevents DB open (#10892).
1112
* Fixed a BackupEngine bug in which RestoreDBFromLatestBackup would fail if the latest backup was deleted and there is another valid backup available.
13+
* Fix L0 file misorder corruption caused by ingesting files of overlapping seqnos with memtable entries' through introducing `epoch_number`. Before the fix, `force_consistency_checks=true` may catch the corruption before it's exposed to readers, in which case writes returning `Status::Corruption` would be expected. Also replace the previous incomplete fix (#5958) to the same corruption with this new and more complete fix.
1214

1315
## 7.9.0 (11/21/2022)
1416
### Performance Improvements

db/column_family.cc

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,8 @@ ColumnFamilyData::ColumnFamilyData(
565565
allow_2pc_(db_options.allow_2pc),
566566
last_memtable_id_(0),
567567
db_paths_registered_(false),
568-
mempurge_used_(false) {
568+
mempurge_used_(false),
569+
next_epoch_number_(1) {
569570
if (id_ != kDummyColumnFamilyDataId) {
570571
// TODO(cc): RegisterDbPaths can be expensive, considering moving it
571572
// outside of this constructor which might be called with db mutex held.
@@ -1128,12 +1129,9 @@ bool ColumnFamilyData::NeedsCompaction() const {
11281129
Compaction* ColumnFamilyData::PickCompaction(
11291130
const MutableCFOptions& mutable_options,
11301131
const MutableDBOptions& mutable_db_options, LogBuffer* log_buffer) {
1131-
SequenceNumber earliest_mem_seqno =
1132-
std::min(mem_->GetEarliestSequenceNumber(),
1133-
imm_.current()->GetEarliestSequenceNumber(false));
11341132
auto* result = compaction_picker_->PickCompaction(
11351133
GetName(), mutable_options, mutable_db_options, current_->storage_info(),
1136-
log_buffer, earliest_mem_seqno);
1134+
log_buffer);
11371135
if (result != nullptr) {
11381136
result->SetInputVersion(current_);
11391137
}
@@ -1520,6 +1518,13 @@ FSDirectory* ColumnFamilyData::GetDataDir(size_t path_id) const {
15201518
return data_dirs_[path_id].get();
15211519
}
15221520

1521+
void ColumnFamilyData::RecoverEpochNumbers() {
1522+
assert(current_);
1523+
auto* vstorage = current_->storage_info();
1524+
assert(vstorage);
1525+
vstorage->RecoverEpochNumbers(this);
1526+
}
1527+
15231528
ColumnFamilySet::ColumnFamilySet(const std::string& dbname,
15241529
const ImmutableDBOptions* db_options,
15251530
const FileOptions& file_options,

db/column_family.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,24 @@ class ColumnFamilyData {
533533
void SetMempurgeUsed() { mempurge_used_ = true; }
534534
bool GetMempurgeUsed() { return mempurge_used_; }
535535

536+
// Allocate and return a new epoch number
537+
uint64_t NewEpochNumber() { return next_epoch_number_.fetch_add(1); }
538+
539+
// Get the next epoch number to be assigned
540+
uint64_t GetNextEpochNumber() const { return next_epoch_number_.load(); }
541+
542+
// Set the next epoch number to be assigned
543+
void SetNextEpochNumber(uint64_t next_epoch_number) {
544+
next_epoch_number_.store(next_epoch_number);
545+
}
546+
547+
// Reset the next epoch number to be assigned
548+
void ResetNextEpochNumber() { next_epoch_number_.store(1); }
549+
550+
// Recover the next epoch number of this CF and epoch number
551+
// of its files (if missing)
552+
void RecoverEpochNumbers();
553+
536554
private:
537555
friend class ColumnFamilySet;
538556
ColumnFamilyData(uint32_t id, const std::string& name,
@@ -634,6 +652,8 @@ class ColumnFamilyData {
634652
// a Version associated with this CFD
635653
std::shared_ptr<CacheReservationManager> file_metadata_cache_res_mgr_;
636654
bool mempurge_used_;
655+
656+
std::atomic<uint64_t> next_epoch_number_;
637657
};
638658

639659
// ColumnFamilySet has interesting thread-safety requirements

db/compaction/compaction.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,16 @@ uint64_t Compaction::MinInputFileOldestAncesterTime(
780780
return min_oldest_ancester_time;
781781
}
782782

783+
uint64_t Compaction::MinInputFileEpochNumber() const {
784+
uint64_t min_epoch_number = std::numeric_limits<uint64_t>::max();
785+
for (const auto& inputs_per_level : inputs_) {
786+
for (const auto& file : inputs_per_level.files) {
787+
min_epoch_number = std::min(min_epoch_number, file->epoch_number);
788+
}
789+
}
790+
return min_epoch_number;
791+
}
792+
783793
int Compaction::EvaluatePenultimateLevel(
784794
const VersionStorageInfo* vstorage,
785795
const ImmutableOptions& immutable_options, const int start_level,

db/compaction/compaction.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ class Compaction {
378378
// This is used to filter out some input files' ancester's time range.
379379
uint64_t MinInputFileOldestAncesterTime(const InternalKey* start,
380380
const InternalKey* end) const;
381+
// Return the minimum epoch number among
382+
// input files' associated with this compaction
383+
uint64_t MinInputFileEpochNumber() const;
381384

382385
// Called by DBImpl::NotifyOnCompactionCompleted to make sure number of
383386
// compaction begin and compaction completion callbacks match.

db/compaction/compaction_job.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1834,12 +1834,14 @@ Status CompactionJob::OpenCompactionOutputFile(SubcompactionState* sub_compact,
18341834
}
18351835

18361836
// Initialize a SubcompactionState::Output and add it to sub_compact->outputs
1837+
uint64_t epoch_number = sub_compact->compaction->MinInputFileEpochNumber();
18371838
{
18381839
FileMetaData meta;
18391840
meta.fd = FileDescriptor(file_number,
18401841
sub_compact->compaction->output_path_id(), 0);
18411842
meta.oldest_ancester_time = oldest_ancester_time;
18421843
meta.file_creation_time = current_time;
1844+
meta.epoch_number = epoch_number;
18431845
meta.temperature = temperature;
18441846
assert(!db_id_.empty());
18451847
assert(!db_session_id_.empty());

db/compaction/compaction_job.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,7 @@ struct CompactionServiceOutputFile {
402402
std::string largest_internal_key;
403403
uint64_t oldest_ancester_time;
404404
uint64_t file_creation_time;
405+
uint64_t epoch_number;
405406
uint64_t paranoid_hash;
406407
bool marked_for_compaction;
407408
UniqueId64x2 unique_id;
@@ -411,15 +412,16 @@ struct CompactionServiceOutputFile {
411412
const std::string& name, SequenceNumber smallest, SequenceNumber largest,
412413
std::string _smallest_internal_key, std::string _largest_internal_key,
413414
uint64_t _oldest_ancester_time, uint64_t _file_creation_time,
414-
uint64_t _paranoid_hash, bool _marked_for_compaction,
415-
UniqueId64x2 _unique_id)
415+
uint64_t _epoch_number, uint64_t _paranoid_hash,
416+
bool _marked_for_compaction, UniqueId64x2 _unique_id)
416417
: file_name(name),
417418
smallest_seqno(smallest),
418419
largest_seqno(largest),
419420
smallest_internal_key(std::move(_smallest_internal_key)),
420421
largest_internal_key(std::move(_largest_internal_key)),
421422
oldest_ancester_time(_oldest_ancester_time),
422423
file_creation_time(_file_creation_time),
424+
epoch_number(_epoch_number),
423425
paranoid_hash(_paranoid_hash),
424426
marked_for_compaction(_marked_for_compaction),
425427
unique_id(std::move(_unique_id)) {}

db/compaction/compaction_job_test.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -380,11 +380,13 @@ class CompactionJobTestBase : public testing::Test {
380380
}
381381

382382
VersionEdit edit;
383-
edit.AddFile(level, file_number, 0, file_size, smallest_key, largest_key,
384-
smallest_seqno, largest_seqno, false, Temperature::kUnknown,
385-
oldest_blob_file_number, kUnknownOldestAncesterTime,
386-
kUnknownFileCreationTime, kUnknownFileChecksum,
387-
kUnknownFileChecksumFuncName, kNullUniqueId64x2);
383+
edit.AddFile(
384+
level, file_number, 0, file_size, smallest_key, largest_key,
385+
smallest_seqno, largest_seqno, false, Temperature::kUnknown,
386+
oldest_blob_file_number, kUnknownOldestAncesterTime,
387+
kUnknownFileCreationTime,
388+
versions_->GetColumnFamilySet()->GetDefault()->NewEpochNumber(),
389+
kUnknownFileChecksum, kUnknownFileChecksumFuncName, kNullUniqueId64x2);
388390

389391
mutex_.Lock();
390392
EXPECT_OK(
@@ -1655,7 +1657,7 @@ TEST_F(CompactionJobTest, ResultSerialization) {
16551657
rnd.RandomBinaryString(rnd.Uniform(kStrMaxLen)),
16561658
rnd.RandomBinaryString(rnd.Uniform(kStrMaxLen)),
16571659
rnd64.Uniform(UINT64_MAX), rnd64.Uniform(UINT64_MAX),
1658-
rnd64.Uniform(UINT64_MAX), rnd.OneIn(2), id);
1660+
rnd64.Uniform(UINT64_MAX), rnd64.Uniform(UINT64_MAX), rnd.OneIn(2), id);
16591661
}
16601662
result.output_level = rnd.Uniform(10);
16611663
result.output_path = rnd.RandomString(rnd.Uniform(kStrMaxLen));

db/compaction/compaction_picker.cc

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,27 +31,15 @@ bool FindIntraL0Compaction(const std::vector<FileMetaData*>& level_files,
3131
size_t min_files_to_compact,
3232
uint64_t max_compact_bytes_per_del_file,
3333
uint64_t max_compaction_bytes,
34-
CompactionInputFiles* comp_inputs,
35-
SequenceNumber earliest_mem_seqno) {
36-
// Do not pick ingested file when there is at least one memtable not flushed
37-
// which of seqno is overlap with the sst.
34+
CompactionInputFiles* comp_inputs) {
3835
TEST_SYNC_POINT("FindIntraL0Compaction");
36+
3937
size_t start = 0;
40-
for (; start < level_files.size(); start++) {
41-
if (level_files[start]->being_compacted) {
42-
return false;
43-
}
44-
// If there is no data in memtable, the earliest sequence number would the
45-
// largest sequence number in last memtable.
46-
// Because all files are sorted in descending order by largest_seqno, so we
47-
// only need to check the first one.
48-
if (level_files[start]->fd.largest_seqno <= earliest_mem_seqno) {
49-
break;
50-
}
51-
}
52-
if (start >= level_files.size()) {
38+
39+
if (level_files.size() == 0 || level_files[start]->being_compacted) {
5340
return false;
5441
}
42+
5543
size_t compact_bytes = static_cast<size_t>(level_files[start]->fd.file_size);
5644
size_t compact_bytes_per_del_file = std::numeric_limits<size_t>::max();
5745
// Compaction range will be [start, limit).
@@ -995,6 +983,7 @@ Status CompactionPicker::SanitizeCompactionInputFilesForAllLevels(
995983
current_files[f].name +
996984
" is currently being compacted.");
997985
}
986+
998987
input_files->insert(TableFileNameToNumber(current_files[f].name));
999988
}
1000989

db/compaction/compaction_picker.h

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,15 @@ class CompactionPicker {
5151
virtual ~CompactionPicker();
5252

5353
// Pick level and inputs for a new compaction.
54+
//
5455
// Returns nullptr if there is no compaction to be done.
5556
// Otherwise returns a pointer to a heap-allocated object that
5657
// describes the compaction. Caller should delete the result.
57-
virtual Compaction* PickCompaction(
58-
const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
59-
const MutableDBOptions& mutable_db_options, VersionStorageInfo* vstorage,
60-
LogBuffer* log_buffer,
61-
SequenceNumber earliest_memtable_seqno = kMaxSequenceNumber) = 0;
58+
virtual Compaction* PickCompaction(const std::string& cf_name,
59+
const MutableCFOptions& mutable_cf_options,
60+
const MutableDBOptions& mutable_db_options,
61+
VersionStorageInfo* vstorage,
62+
LogBuffer* log_buffer) = 0;
6263

6364
// Return a compaction object for compacting the range [begin,end] in
6465
// the specified level. Returns nullptr if there is nothing in that
@@ -91,6 +92,7 @@ class CompactionPicker {
9192
// files. If it's not possible to conver an invalid input_files
9293
// into a valid one by adding more files, the function will return a
9394
// non-ok status with specific reason.
95+
//
9496
#ifndef ROCKSDB_LITE
9597
Status SanitizeCompactionInputFiles(std::unordered_set<uint64_t>* input_files,
9698
const ColumnFamilyMetaData& cf_meta,
@@ -255,12 +257,11 @@ class NullCompactionPicker : public CompactionPicker {
255257
virtual ~NullCompactionPicker() {}
256258

257259
// Always return "nullptr"
258-
Compaction* PickCompaction(
259-
const std::string& /*cf_name*/,
260-
const MutableCFOptions& /*mutable_cf_options*/,
261-
const MutableDBOptions& /*mutable_db_options*/,
262-
VersionStorageInfo* /*vstorage*/, LogBuffer* /* log_buffer */,
263-
SequenceNumber /* earliest_memtable_seqno */) override {
260+
Compaction* PickCompaction(const std::string& /*cf_name*/,
261+
const MutableCFOptions& /*mutable_cf_options*/,
262+
const MutableDBOptions& /*mutable_db_options*/,
263+
VersionStorageInfo* /*vstorage*/,
264+
LogBuffer* /* log_buffer */) override {
264265
return nullptr;
265266
}
266267

@@ -304,11 +305,11 @@ class NullCompactionPicker : public CompactionPicker {
304305
// files. Cannot be nullptr.
305306
//
306307
// @return true iff compaction was found.
307-
bool FindIntraL0Compaction(
308-
const std::vector<FileMetaData*>& level_files, size_t min_files_to_compact,
309-
uint64_t max_compact_bytes_per_del_file, uint64_t max_compaction_bytes,
310-
CompactionInputFiles* comp_inputs,
311-
SequenceNumber earliest_mem_seqno = kMaxSequenceNumber);
308+
bool FindIntraL0Compaction(const std::vector<FileMetaData*>& level_files,
309+
size_t min_files_to_compact,
310+
uint64_t max_compact_bytes_per_del_file,
311+
uint64_t max_compaction_bytes,
312+
CompactionInputFiles* comp_inputs);
312313

313314
CompressionType GetCompressionType(const VersionStorageInfo* vstorage,
314315
const MutableCFOptions& mutable_cf_options,

db/compaction/compaction_picker_fifo.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ Compaction* FIFOCompactionPicker::PickCompactionToWarm(
402402
Compaction* FIFOCompactionPicker::PickCompaction(
403403
const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
404404
const MutableDBOptions& mutable_db_options, VersionStorageInfo* vstorage,
405-
LogBuffer* log_buffer, SequenceNumber /*earliest_memtable_seqno*/) {
405+
LogBuffer* log_buffer) {
406406
Compaction* c = nullptr;
407407
if (mutable_cf_options.ttl > 0) {
408408
c = PickTTLCompaction(cf_name, mutable_cf_options, mutable_db_options,

db/compaction/compaction_picker_fifo.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ class FIFOCompactionPicker : public CompactionPicker {
1919
const InternalKeyComparator* icmp)
2020
: CompactionPicker(ioptions, icmp) {}
2121

22-
virtual Compaction* PickCompaction(
23-
const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
24-
const MutableDBOptions& mutable_db_options, VersionStorageInfo* version,
25-
LogBuffer* log_buffer,
26-
SequenceNumber earliest_memtable_seqno = kMaxSequenceNumber) override;
22+
virtual Compaction* PickCompaction(const std::string& cf_name,
23+
const MutableCFOptions& mutable_cf_options,
24+
const MutableDBOptions& mutable_db_options,
25+
VersionStorageInfo* version,
26+
LogBuffer* log_buffer) override;
2727

2828
virtual Compaction* CompactRange(
2929
const std::string& cf_name, const MutableCFOptions& mutable_cf_options,

db/compaction/compaction_picker_level.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,13 @@ class LevelCompactionBuilder {
5050
public:
5151
LevelCompactionBuilder(const std::string& cf_name,
5252
VersionStorageInfo* vstorage,
53-
SequenceNumber earliest_mem_seqno,
5453
CompactionPicker* compaction_picker,
5554
LogBuffer* log_buffer,
5655
const MutableCFOptions& mutable_cf_options,
5756
const ImmutableOptions& ioptions,
5857
const MutableDBOptions& mutable_db_options)
5958
: cf_name_(cf_name),
6059
vstorage_(vstorage),
61-
earliest_mem_seqno_(earliest_mem_seqno),
6260
compaction_picker_(compaction_picker),
6361
log_buffer_(log_buffer),
6462
mutable_cf_options_(mutable_cf_options),
@@ -122,7 +120,6 @@ class LevelCompactionBuilder {
122120

123121
const std::string& cf_name_;
124122
VersionStorageInfo* vstorage_;
125-
SequenceNumber earliest_mem_seqno_;
126123
CompactionPicker* compaction_picker_;
127124
LogBuffer* log_buffer_;
128125
int start_level_ = -1;
@@ -196,7 +193,10 @@ void LevelCompactionBuilder::SetupInitialFiles() {
196193
}
197194
output_level_ =
198195
(start_level_ == 0) ? vstorage_->base_level() : start_level_ + 1;
199-
if (PickFileToCompact()) {
196+
bool picked_file_to_compact = PickFileToCompact();
197+
TEST_SYNC_POINT_CALLBACK("PostPickFileToCompact",
198+
&picked_file_to_compact);
199+
if (picked_file_to_compact) {
200200
// found the compaction!
201201
if (start_level_ == 0) {
202202
// L0 score = `num L0 files` / `level0_file_num_compaction_trigger`
@@ -825,16 +825,16 @@ bool LevelCompactionBuilder::PickIntraL0Compaction() {
825825
return FindIntraL0Compaction(level_files, kMinFilesForIntraL0Compaction,
826826
std::numeric_limits<uint64_t>::max(),
827827
mutable_cf_options_.max_compaction_bytes,
828-
&start_level_inputs_, earliest_mem_seqno_);
828+
&start_level_inputs_);
829829
}
830830
} // namespace
831831

832832
Compaction* LevelCompactionPicker::PickCompaction(
833833
const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
834834
const MutableDBOptions& mutable_db_options, VersionStorageInfo* vstorage,
835-
LogBuffer* log_buffer, SequenceNumber earliest_mem_seqno) {
836-
LevelCompactionBuilder builder(cf_name, vstorage, earliest_mem_seqno, this,
837-
log_buffer, mutable_cf_options, ioptions_,
835+
LogBuffer* log_buffer) {
836+
LevelCompactionBuilder builder(cf_name, vstorage, this, log_buffer,
837+
mutable_cf_options, ioptions_,
838838
mutable_db_options);
839839
return builder.PickCompaction();
840840
}

db/compaction/compaction_picker_level.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ class LevelCompactionPicker : public CompactionPicker {
2020
LevelCompactionPicker(const ImmutableOptions& ioptions,
2121
const InternalKeyComparator* icmp)
2222
: CompactionPicker(ioptions, icmp) {}
23-
virtual Compaction* PickCompaction(
24-
const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
25-
const MutableDBOptions& mutable_db_options, VersionStorageInfo* vstorage,
26-
LogBuffer* log_buffer,
27-
SequenceNumber earliest_memtable_seqno = kMaxSequenceNumber) override;
23+
virtual Compaction* PickCompaction(const std::string& cf_name,
24+
const MutableCFOptions& mutable_cf_options,
25+
const MutableDBOptions& mutable_db_options,
26+
VersionStorageInfo* vstorage,
27+
LogBuffer* log_buffer) override;
2828

2929
virtual bool NeedsCompaction(
3030
const VersionStorageInfo* vstorage) const override;

0 commit comments

Comments
 (0)