Skip to content

Commit 6e4113e

Browse files
hx235facebook-github-bot
hx235
authored and
facebook-github-bot
committed
Remove reductant Compaction parameters (facebook#13777)
Summary: **Context/Summary:** a small refactoring to make Compaction constructor simpler (though still complicated now). Pull Request resolved: facebook#13777 Test Plan: Existing tests Reviewed By: jaykorean Differential Revision: D78385166 Pulled By: hx235 fbshipit-source-id: cd93d1ba3936d9f9077ffceb0dc4ef5506e51017
1 parent 0be850a commit 6e4113e

9 files changed

Lines changed: 56 additions & 72 deletions

‎db/compaction/compaction.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,9 @@ Compaction::Compaction(
284284
CompressionOptions _compression_opts, Temperature _output_temperature,
285285
uint32_t _max_subcompactions, std::vector<FileMetaData*> _grandparents,
286286
std::optional<SequenceNumber> _earliest_snapshot,
287-
const SnapshotChecker* _snapshot_checker, bool _manual_compaction,
288-
const std::string& _trim_ts, double _score, bool _deletion_compaction,
289-
bool l0_files_might_overlap, CompactionReason _compaction_reason,
287+
const SnapshotChecker* _snapshot_checker,
288+
CompactionReason _compaction_reason, const std::string& _trim_ts,
289+
double _score, bool l0_files_might_overlap,
290290
BlobGarbageCollectionPolicy _blob_garbage_collection_policy,
291291
double _blob_garbage_collection_age_cutoff)
292292
: input_vstorage_(vstorage),
@@ -304,7 +304,9 @@ Compaction::Compaction(
304304
output_compression_(_compression),
305305
output_compression_opts_(_compression_opts),
306306
output_temperature_(_output_temperature),
307-
deletion_compaction_(_deletion_compaction),
307+
deletion_compaction_(_compaction_reason == CompactionReason::kFIFOTtl ||
308+
_compaction_reason ==
309+
CompactionReason::kFIFOMaxSize),
308310
l0_files_might_overlap_(l0_files_might_overlap),
309311
inputs_(PopulateWithAtomicBoundaries(vstorage, std::move(_inputs))),
310312
grandparents_(std::move(_grandparents)),
@@ -321,7 +323,8 @@ Compaction::Compaction(
321323
? false
322324
: IsBottommostLevel(output_level_, vstorage, inputs_)),
323325
is_full_compaction_(IsFullCompaction(vstorage, inputs_)),
324-
is_manual_compaction_(_manual_compaction),
326+
is_manual_compaction_(_compaction_reason ==
327+
CompactionReason::kManualCompaction),
325328
trim_ts_(_trim_ts),
326329
is_trivial_move_(false),
327330
compaction_reason_(_compaction_reason),
@@ -349,9 +352,6 @@ Compaction::Compaction(
349352
immutable_options_, start_level_,
350353
output_level_)) {
351354
MarkFilesBeingCompacted(true);
352-
if (is_manual_compaction_) {
353-
compaction_reason_ = CompactionReason::kManualCompaction;
354-
}
355355
if (max_subcompactions_ == 0) {
356356
max_subcompactions_ = _mutable_db_options.max_subcompactions;
357357
}

‎db/compaction/compaction.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,9 @@ class Compaction {
9494
std::vector<FileMetaData*> grandparents,
9595
std::optional<SequenceNumber> earliest_snapshot,
9696
const SnapshotChecker* snapshot_checker,
97-
bool manual_compaction = false, const std::string& trim_ts = "",
98-
double score = -1, bool deletion_compaction = false,
97+
CompactionReason compaction_reason,
98+
const std::string& trim_ts = "", double score = -1,
9999
bool l0_files_might_overlap = true,
100-
CompactionReason compaction_reason = CompactionReason::kUnknown,
101100
BlobGarbageCollectionPolicy blob_garbage_collection_policy =
102101
BlobGarbageCollectionPolicy::kUseDefault,
103102
double blob_garbage_collection_age_cutoff = -1);

‎db/compaction/compaction_job_test.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,8 @@ class CompactionJobTestBase : public testing::Test {
651651
mutable_cf_options_.max_compaction_bytes, 0, kNoCompression,
652652
cfd->GetLatestMutableCFOptions().compression_opts,
653653
Temperature::kUnknown, max_subcompactions, grandparents,
654-
/*earliest_snapshot*/ std::nullopt, /*snapshot_checker*/ nullptr, true);
654+
/*earliest_snapshot*/ std::nullopt, /*snapshot_checker*/ nullptr,
655+
CompactionReason::kManualCompaction);
655656
compaction.FinalizeInputInfo(cfd->current());
656657

657658
assert(db_options_.info_log);

‎db/compaction/compaction_picker.cc‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ Compaction* CompactionPicker::CompactFiles(
381381
mutable_cf_options.default_write_temperature,
382382
compact_options.max_subcompactions,
383383
/* grandparents */ {}, /* earliest_snapshot */ std::nullopt,
384-
/* snapshot_checker */ nullptr, true);
384+
/* snapshot_checker */ nullptr, CompactionReason::kManualCompaction);
385385
RegisterCompaction(c);
386386
return c;
387387
}
@@ -680,10 +680,9 @@ Compaction* CompactionPicker::CompactRange(
680680
mutable_cf_options.default_write_temperature,
681681
compact_range_options.max_subcompactions,
682682
/* grandparents */ {}, /* earliest_snapshot */ std::nullopt,
683-
/* snapshot_checker */ nullptr,
684-
/* is manual */ true, trim_ts, /* score */ -1,
685-
/* deletion_compaction */ false, /* l0_files_might_overlap */ true,
686-
CompactionReason::kUnknown,
683+
/* snapshot_checker */ nullptr, CompactionReason::kManualCompaction,
684+
trim_ts, /* score */ -1,
685+
/* l0_files_might_overlap */ true,
687686
compact_range_options.blob_garbage_collection_policy,
688687
compact_range_options.blob_garbage_collection_age_cutoff);
689688

@@ -873,9 +872,8 @@ Compaction* CompactionPicker::CompactRange(
873872
mutable_cf_options.default_write_temperature,
874873
compact_range_options.max_subcompactions, std::move(grandparents),
875874
/* earliest_snapshot */ std::nullopt, /* snapshot_checker */ nullptr,
876-
/* is manual */ true, trim_ts, /* score */ -1,
877-
/* deletion_compaction */ false, /* l0_files_might_overlap */ true,
878-
CompactionReason::kUnknown,
875+
CompactionReason::kManualCompaction, trim_ts, /* score */ -1,
876+
/* l0_files_might_overlap */ true,
879877
compact_range_options.blob_garbage_collection_policy,
880878
compact_range_options.blob_garbage_collection_age_cutoff);
881879

‎db/compaction/compaction_picker_fifo.cc‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,9 @@ Compaction* FIFOCompactionPicker::PickTTLCompaction(
127127
mutable_cf_options.compression_opts,
128128
mutable_cf_options.default_write_temperature,
129129
/* max_subcompactions */ 0, {}, /* earliest_snapshot */ std::nullopt,
130-
/* snapshot_checker */ nullptr,
131-
/* is manual */ false,
130+
/* snapshot_checker */ nullptr, CompactionReason::kFIFOTtl,
132131
/* trim_ts */ "", vstorage->CompactionScore(0),
133-
/* is deletion compaction */ true, /* l0_files_might_overlap */ true,
134-
CompactionReason::kFIFOTtl);
132+
/* l0_files_might_overlap */ true);
135133
return c;
136134
}
137135

@@ -200,11 +198,10 @@ Compaction* FIFOCompactionPicker::PickSizeCompaction(
200198
mutable_cf_options.default_write_temperature,
201199
0 /* max_subcompactions */, {},
202200
/* earliest_snapshot */ std::nullopt,
203-
/* snapshot_checker */ nullptr, /* is manual */ false,
201+
/* snapshot_checker */ nullptr,
202+
CompactionReason::kFIFOReduceNumFiles,
204203
/* trim_ts */ "", vstorage->CompactionScore(0),
205-
/* is deletion compaction */ false,
206-
/* l0_files_might_overlap */ true,
207-
CompactionReason::kFIFOReduceNumFiles);
204+
/* l0_files_might_overlap */ true);
208205
return c;
209206
}
210207
}
@@ -297,11 +294,9 @@ Compaction* FIFOCompactionPicker::PickSizeCompaction(
297294
mutable_cf_options.compression_opts,
298295
mutable_cf_options.default_write_temperature,
299296
/* max_subcompactions */ 0, {}, /* earliest_snapshot */ std::nullopt,
300-
/* snapshot_checker */ nullptr,
301-
/* is manual */ false,
297+
/* snapshot_checker */ nullptr, CompactionReason::kFIFOMaxSize,
302298
/* trim_ts */ "", vstorage->CompactionScore(0),
303-
/* is deletion compaction */ true,
304-
/* l0_files_might_overlap */ true, CompactionReason::kFIFOMaxSize);
299+
/* l0_files_might_overlap */ true);
305300
return c;
306301
}
307302

@@ -416,10 +411,9 @@ Compaction* FIFOCompactionPicker::PickTemperatureChangeCompaction(
416411
mutable_cf_options.compression, mutable_cf_options.compression_opts,
417412
compaction_target_temp,
418413
/* max_subcompactions */ 0, {}, /* earliest_snapshot */ std::nullopt,
419-
/* snapshot_checker */ nullptr,
420-
/* is manual */ false, /* trim_ts */ "", vstorage->CompactionScore(0),
421-
/* is deletion compaction */ false, /* l0_files_might_overlap */ true,
422-
CompactionReason::kChangeTemperature);
414+
/* snapshot_checker */ nullptr, CompactionReason::kChangeTemperature,
415+
/* trim_ts */ "", vstorage->CompactionScore(0),
416+
/* l0_files_might_overlap */ true);
423417
return c;
424418
}
425419

‎db/compaction/compaction_picker_level.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ class LevelCompactionBuilder {
145145
int parent_index_ = -1;
146146
int base_index_ = -1;
147147
double start_level_score_ = 0;
148-
bool is_manual_ = false;
149148
bool is_l0_trivial_move_ = false;
150149
CompactionInputFiles start_level_inputs_;
151150
std::vector<CompactionInputFiles> compaction_inputs_;
@@ -561,9 +560,8 @@ Compaction* LevelCompactionBuilder::GetCompaction() {
561560
mutable_cf_options_.default_write_temperature,
562561
/* max_subcompactions */ 0, std::move(grandparents_),
563562
/* earliest_snapshot */ std::nullopt, /* snapshot_checker */ nullptr,
564-
is_manual_,
565-
/* trim_ts */ "", start_level_score_, false /* deletion_compaction */,
566-
l0_files_might_overlap, compaction_reason_);
563+
compaction_reason_,
564+
/* trim_ts */ "", start_level_score_, l0_files_might_overlap);
567565

568566
// If it's level 0 compaction, make sure we don't execute any other level 0
569567
// compactions in parallel

‎db/compaction/compaction_picker_universal.cc‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,10 +1097,9 @@ Compaction* UniversalCompactionBuilder::PickCompactionToReduceSortedRuns(
10971097
mutable_cf_options_.default_write_temperature,
10981098
/* max_subcompactions */ 0, grandparents,
10991099
/* earliest_snapshot */ std::nullopt,
1100-
/* snapshot_checker */ nullptr,
1101-
/* is manual */ false, /* trim_ts */ "", score_,
1102-
false /* deletion_compaction */,
1103-
/* l0_files_might_overlap */ true, compaction_reason);
1100+
/* snapshot_checker */ nullptr, compaction_reason,
1101+
/* trim_ts */ "", score_,
1102+
/* l0_files_might_overlap */ true);
11041103
}
11051104

11061105
// Look at overall size amplification. If size amplification
@@ -1447,10 +1446,9 @@ Compaction* UniversalCompactionBuilder::PickIncrementalForReduceSizeAmp(
14471446
/* max_subcompactions */ 0, /* grandparents */ {},
14481447
/* earliest_snapshot */ std::nullopt,
14491448
/* snapshot_checker */ nullptr,
1450-
/* is manual */ false,
1451-
/* trim_ts */ "", score_, false /* deletion_compaction */,
1452-
/* l0_files_might_overlap */ true,
1453-
CompactionReason::kUniversalSizeAmplification);
1449+
CompactionReason::kUniversalSizeAmplification,
1450+
/* trim_ts */ "", score_,
1451+
/* l0_files_might_overlap */ true);
14541452
}
14551453

14561454
// Pick files marked for compaction. Typically, files are marked by
@@ -1600,11 +1598,9 @@ Compaction* UniversalCompactionBuilder::PickDeleteTriggeredCompaction() {
16001598
GetCompressionOptions(mutable_cf_options_, vstorage_, output_level),
16011599
mutable_cf_options_.default_write_temperature,
16021600
/* max_subcompactions */ 0, grandparents, earliest_snapshot_,
1603-
snapshot_checker_,
1604-
/* is manual */ false,
1605-
/* trim_ts */ "", score_, false /* deletion_compaction */,
1606-
/* l0_files_might_overlap */ true,
1607-
CompactionReason::kFilesMarkedForCompaction);
1601+
snapshot_checker_, CompactionReason::kFilesMarkedForCompaction,
1602+
/* trim_ts */ "", score_,
1603+
/* l0_files_might_overlap */ true);
16081604
}
16091605

16101606
Compaction* UniversalCompactionBuilder::PickCompactionToOldest(
@@ -1700,10 +1696,9 @@ Compaction* UniversalCompactionBuilder::PickCompactionWithSortedRunRange(
17001696
mutable_cf_options_.default_write_temperature,
17011697
/* max_subcompactions */ 0, /* grandparents */ {},
17021698
/* earliest_snapshot */ std::nullopt,
1703-
/* snapshot_checker */ nullptr,
1704-
/* is manual */ false,
1705-
/* trim_ts */ "", score_, false /* deletion_compaction */,
1706-
/* l0_files_might_overlap */ true, compaction_reason);
1699+
/* snapshot_checker */ nullptr, compaction_reason,
1700+
/* trim_ts */ "", score_,
1701+
/* l0_files_might_overlap */ true);
17071702
}
17081703

17091704
Compaction* UniversalCompactionBuilder::PickPeriodicCompaction() {

‎db/db_impl/db_impl_compaction_flush.cc‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,10 +1849,9 @@ Status DBImpl::ReFitLevel(ColumnFamilyData* cfd, int level, int target_level) {
18491849
0 /* max_subcompactions, not applicable */,
18501850
{} /* grandparents, not applicable */,
18511851
std::nullopt /* earliest_snapshot */, nullptr /* snapshot_checker */,
1852-
false /* is manual */, "" /* trim_ts */, -1 /* score, not applicable */,
1853-
false /* is deletion compaction, not applicable */,
1854-
false /* l0_files_might_overlap, not applicable */,
1855-
CompactionReason::kRefitLevel));
1852+
CompactionReason::kRefitLevel, "" /* trim_ts */,
1853+
-1 /* score, not applicable */,
1854+
false /* l0_files_might_overlap, not applicable */));
18561855
cfd->compaction_picker()->RegisterCompaction(c.get());
18571856
TEST_SYNC_POINT("DBImpl::ReFitLevel:PostRegisterCompaction");
18581857
VersionEdit edit;
@@ -4424,13 +4423,14 @@ Compaction* DBImpl::CreateIntendedCompactionForwardedToBottomPriorityPool(
44244423

44254424
c->ReleaseCompactionFiles(Status::OK());
44264425

4427-
Compaction* intended_compaction = new Compaction(
4428-
vstorage, io, mo, mutable_db_options_, std::move(inputs),
4429-
c->output_level(), c->target_output_file_size(),
4430-
c->max_compaction_bytes(), c->output_path_id(), c->output_compression(),
4431-
c->output_compression_opts(), c->output_temperature(),
4432-
c->max_subcompactions(), c->grandparents(),
4433-
std::nullopt /* earliest_snapshot */, nullptr /* snapshot_checker */);
4426+
Compaction* intended_compaction =
4427+
new Compaction(vstorage, io, mo, mutable_db_options_, std::move(inputs),
4428+
c->output_level(), c->target_output_file_size(),
4429+
c->max_compaction_bytes(), c->output_path_id(),
4430+
c->output_compression(), c->output_compression_opts(),
4431+
c->output_temperature(), c->max_subcompactions(),
4432+
c->grandparents(), std::nullopt /* earliest_snapshot */,
4433+
nullptr /* snapshot_checker */, c->compaction_reason());
44344434

44354435
cfd->compaction_picker()->RegisterCompaction(intended_compaction);
44364436
vstorage->ComputeCompactionScore(io, mo);

‎db/external_sst_file_ingestion_job.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -701,10 +701,9 @@ void ExternalSstFileIngestionJob::CreateEquivalentFileIngestingCompactions() {
701701
0 /* max_subcompaction, not applicable */,
702702
{} /* grandparents, not applicable */,
703703
std::nullopt /* earliest_snapshot */, nullptr /* snapshot_checker */,
704-
false /* is manual */, "" /* trim_ts */, -1 /* score, not applicable */,
705-
false /* is deletion compaction, not applicable */,
706-
files_overlap_ /* l0_files_might_overlap, not applicable */,
707-
CompactionReason::kExternalSstIngestion));
704+
CompactionReason::kExternalSstIngestion, "" /* trim_ts */,
705+
-1 /* score, not applicable */,
706+
files_overlap_ /* l0_files_might_overlap, not applicable */));
708707
}
709708
}
710709

0 commit comments

Comments
 (0)