Remove deprecated option `ignore_max_compaction_bytes_for_input` (#12323)

Summary:
The option is introduced in https://github.com/facebook/rocksdb/issues/10835 to allow disabling the new compaction behavior if it's not safe. The option is enabled by default and there has not been a need to disable it. So it should be safe to remove now.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/12323

Reviewed By: ajkr

Differential Revision: D53330336

Pulled By: cbi42

fbshipit-source-id: 36eef4664ac96b3a7ed627c48bd6610b0a7eafc5
This commit is contained in:
Changyu Bi 2024-02-02 17:09:42 -08:00 committed by Facebook GitHub Bot
parent ace1721b28
commit 5620efc794
12 changed files with 27 additions and 89 deletions

View File

@ -456,10 +456,9 @@ bool CompactionPicker::IsRangeInCompaction(VersionStorageInfo* vstorage,
// Returns false if files on parent level are currently in compaction, which
// means that we can't compact them
bool CompactionPicker::SetupOtherInputs(
const std::string& cf_name, const MutableCFOptions& mutable_cf_options,
VersionStorageInfo* vstorage, CompactionInputFiles* inputs,
CompactionInputFiles* output_level_inputs, int* parent_index,
int base_index, bool only_expand_towards_right) {
const std::string& cf_name, VersionStorageInfo* vstorage,
CompactionInputFiles* inputs, CompactionInputFiles* output_level_inputs,
int* parent_index, int base_index, bool only_expand_towards_right) {
assert(!inputs->empty());
assert(output_level_inputs->empty());
const int input_level = inputs->level;
@ -500,7 +499,6 @@ bool CompactionPicker::SetupOtherInputs(
// user key, while excluding other entries for the same user key. This
// can happen when one user key spans multiple files.
if (!output_level_inputs->empty()) {
const uint64_t limit = mutable_cf_options.max_compaction_bytes;
const uint64_t output_level_inputs_size =
TotalFileSize(output_level_inputs->files);
const uint64_t inputs_size = TotalFileSize(inputs->files);
@ -527,8 +525,6 @@ bool CompactionPicker::SetupOtherInputs(
try_overlapping_inputs = false;
}
if (try_overlapping_inputs && expanded_inputs.size() > inputs->size() &&
(mutable_cf_options.ignore_max_compaction_bytes_for_input ||
output_level_inputs_size + expanded_inputs_size < limit) &&
!AreFilesInCompaction(expanded_inputs.files)) {
InternalKey new_start, new_limit;
GetRange(expanded_inputs, &new_start, &new_limit);
@ -551,8 +547,6 @@ bool CompactionPicker::SetupOtherInputs(
base_index, nullptr);
expanded_inputs_size = TotalFileSize(expanded_inputs.files);
if (expanded_inputs.size() > inputs->size() &&
(mutable_cf_options.ignore_max_compaction_bytes_for_input ||
output_level_inputs_size + expanded_inputs_size < limit) &&
!AreFilesInCompaction(expanded_inputs.files)) {
expand_inputs = true;
}
@ -812,8 +806,8 @@ Compaction* CompactionPicker::CompactRange(
output_level_inputs.level = output_level;
if (input_level != output_level) {
int parent_index = -1;
if (!SetupOtherInputs(cf_name, mutable_cf_options, vstorage, &inputs,
&output_level_inputs, &parent_index, -1)) {
if (!SetupOtherInputs(cf_name, vstorage, &inputs, &output_level_inputs,
&parent_index, -1)) {
// manual compaction is now multi-threaded, so it can
// happen that SetupOtherInputs fails
// we handle it higher in RunManualCompaction

View File

@ -186,7 +186,6 @@ class CompactionPicker {
int penultimate_level) const;
bool SetupOtherInputs(const std::string& cf_name,
const MutableCFOptions& mutable_cf_options,
VersionStorageInfo* vstorage,
CompactionInputFiles* inputs,
CompactionInputFiles* output_level_inputs,

View File

@ -465,9 +465,8 @@ bool LevelCompactionBuilder::SetupOtherInputsIfNeeded() {
}
if (!is_l0_trivial_move_ &&
!compaction_picker_->SetupOtherInputs(
cf_name_, mutable_cf_options_, vstorage_, &start_level_inputs_,
&output_level_inputs_, &parent_index_, base_index_,
round_robin_expanding)) {
cf_name_, vstorage_, &start_level_inputs_, &output_level_inputs_,
&parent_index_, base_index_, round_robin_expanding)) {
return false;
}

View File

@ -1443,15 +1443,12 @@ TEST_F(CompactionPickerTest, CompactionPriMinOverlapping4) {
ioptions_.compaction_pri = kMinOverlappingRatio;
mutable_cf_options_.max_bytes_for_level_base = 10000000;
mutable_cf_options_.max_bytes_for_level_multiplier = 10;
mutable_cf_options_.ignore_max_compaction_bytes_for_input = false;
// file 7 and 8 over lap with the same file, but file 8 is smaller so
// it will be picked.
// Overlaps with file 26, 27. And the file is compensated so will be
// picked up.
// Overlaps with file 26, 27, ratio is
// (60000000U + 60000000U / 180000000U) = 0.67
Add(2, 6U, "150", "167", 60000000U, 0, 100, 100, 180000000U);
Add(2, 7U, "168", "169", 60000000U); // Overlaps with file 27
Add(2, 8U, "201", "300", 61000000U); // Overlaps with file 28
Add(2, 7U, "178", "189", 60000000U); // Overlaps with file 28
Add(2, 8U, "401", "500", 61000000U); // Overlaps with file 29
Add(3, 26U, "160", "165", 60000000U);
// Boosted file size in output level is not considered.
@ -1465,7 +1462,7 @@ TEST_F(CompactionPickerTest, CompactionPriMinOverlapping4) {
&log_buffer_));
ASSERT_TRUE(compaction.get() != nullptr);
ASSERT_EQ(1U, compaction->num_input_files(0));
// Picking file 8 because overlapping ratio is the biggest.
// Picking file 6 because overlapping ratio is the biggest.
ASSERT_EQ(6U, compaction->input(0, 0)->fd.GetNumber());
}
@ -2468,20 +2465,23 @@ TEST_F(CompactionPickerTest, IsBottommostLevelTest) {
DeleteVersionStorage();
}
TEST_F(CompactionPickerTest, MaxCompactionBytesHit) {
TEST_F(CompactionPickerTest, IgnoreCompactionLimitWhenAddFileFromInputLevel) {
mutable_cf_options_.max_bytes_for_level_base = 1000000u;
mutable_cf_options_.max_compaction_bytes = 800000u;
mutable_cf_options_.ignore_max_compaction_bytes_for_input = false;
ioptions_.level_compaction_dynamic_level_bytes = false;
NewVersionStorage(6, kCompactionStyleLevel);
// A compaction should be triggered and pick file 2 and 5.
// It can expand because adding file 1 and 3, the compaction size will
// exceed mutable_cf_options_.max_bytes_for_level_base.
// It pulls in other compaction input file from the input level L1
// without pulling in more output level files.
// Files 1, 3, 4 will be included in the compaction.
// File 6 is excluded since it overlaps with file 7.
Add(1, 1U, "100", "150", 300000U);
Add(1, 2U, "151", "200", 300001U, 0, 0);
Add(1, 3U, "201", "250", 300000U, 0, 0);
Add(1, 4U, "251", "300", 300000U, 0, 0);
Add(2, 5U, "100", "256", 1U);
Add(1, 6U, "325", "400", 300000U, 0, 0);
Add(2, 5U, "100", "350", 1U);
Add(2, 7U, "375", "425", 1U);
UpdateVersionStorageInfo();
std::unique_ptr<Compaction> compaction(level_compaction_picker.PickCompaction(
@ -2489,37 +2489,12 @@ TEST_F(CompactionPickerTest, MaxCompactionBytesHit) {
&log_buffer_));
ASSERT_TRUE(compaction.get() != nullptr);
ASSERT_EQ(2U, compaction->num_input_levels());
ASSERT_EQ(1U, compaction->num_input_files(0));
ASSERT_EQ(1U, compaction->num_input_files(1));
ASSERT_EQ(2U, compaction->input(0, 0)->fd.GetNumber());
ASSERT_EQ(5U, compaction->input(1, 0)->fd.GetNumber());
}
TEST_F(CompactionPickerTest, MaxCompactionBytesNotHit) {
mutable_cf_options_.max_bytes_for_level_base = 800000u;
mutable_cf_options_.max_compaction_bytes = 1000000u;
mutable_cf_options_.ignore_max_compaction_bytes_for_input = false;
ioptions_.level_compaction_dynamic_level_bytes = false;
NewVersionStorage(6, kCompactionStyleLevel);
// A compaction should be triggered and pick file 2 and 5.
// and it expands to file 1 and 3 too.
Add(1, 1U, "100", "150", 300000U);
Add(1, 2U, "151", "200", 300001U, 0, 0);
Add(1, 3U, "201", "250", 300000U, 0, 0);
Add(1, 4U, "251", "300", 300000U, 0, 0);
Add(2, 5U, "000", "251", 1U);
UpdateVersionStorageInfo();
std::unique_ptr<Compaction> compaction(level_compaction_picker.PickCompaction(
cf_name_, mutable_cf_options_, mutable_db_options_, vstorage_.get(),
&log_buffer_));
ASSERT_TRUE(compaction.get() != nullptr);
ASSERT_EQ(2U, compaction->num_input_levels());
ASSERT_EQ(3U, compaction->num_input_files(0));
ASSERT_EQ(4U, compaction->num_input_files(0));
ASSERT_EQ(1U, compaction->num_input_files(1));
ASSERT_EQ(1U, compaction->input(0, 0)->fd.GetNumber());
ASSERT_EQ(2U, compaction->input(0, 1)->fd.GetNumber());
ASSERT_EQ(3U, compaction->input(0, 2)->fd.GetNumber());
ASSERT_EQ(4U, compaction->input(0, 3)->fd.GetNumber());
ASSERT_EQ(5U, compaction->input(1, 0)->fd.GetNumber());
}

View File

@ -1139,8 +1139,7 @@ Compaction* UniversalCompactionBuilder::PickIncrementalForReduceSizeAmp(
// from bottom_start_idx and bottom_end_idx, but for now, we use
// SetupOtherInputs() for simplicity.
int parent_index = -1; // Create and use bottom_start_idx?
if (!picker_->SetupOtherInputs(cf_name_, mutable_cf_options_, vstorage_,
&second_last_level_inputs,
if (!picker_->SetupOtherInputs(cf_name_, vstorage_, &second_last_level_inputs,
&bottom_level_inputs, &parent_index,
/*base_index=*/-1)) {
return nullptr;
@ -1311,9 +1310,8 @@ Compaction* UniversalCompactionBuilder::PickDeleteTriggeredCompaction() {
int parent_index = -1;
output_level_inputs.level = output_level;
if (!picker_->SetupOtherInputs(cf_name_, mutable_cf_options_, vstorage_,
&start_level_inputs, &output_level_inputs,
&parent_index, -1)) {
if (!picker_->SetupOtherInputs(cf_name_, vstorage_, &start_level_inputs,
&output_level_inputs, &parent_index, -1)) {
return nullptr;
}
inputs.push_back(start_level_inputs);

View File

@ -1823,7 +1823,6 @@ TEST_P(PrecludeLastLevelTestWithParms, PeriodicCompactionToPenultimateLevel) {
options.env = mock_env_.get();
options.level0_file_num_compaction_trigger = kNumTrigger;
options.num_levels = kNumLevels;
options.ignore_max_compaction_bytes_for_input = false;
options.periodic_compaction_seconds = 10000;
DestroyAndReopen(options);

View File

@ -573,19 +573,6 @@ struct AdvancedColumnFamilyOptions {
// Dynamically changeable through SetOptions() API
uint64_t max_compaction_bytes = 0;
// DEPRECATED: This option might be removed in a future release.
//
// When setting up compaction input files, we ignore the
// `max_compaction_bytes` limit when pulling in input files that are entirely
// within output key range.
//
// Default: true
//
// Dynamically changeable through SetOptions() API
// We could remove this knob and always ignore the limit once it is proven
// safe.
bool ignore_max_compaction_bytes_for_input = true;
// All writes will be slowed down to at least delayed_write_rate if estimated
// bytes needed to be compaction exceed this threshold.
//

View File

@ -296,9 +296,7 @@ static std::unordered_map<std::string, OptionTypeInfo>
OptionType::kUInt64T, OptionVerificationType::kNormal,
OptionTypeFlags::kMutable}},
{"ignore_max_compaction_bytes_for_input",
{offsetof(struct MutableCFOptions,
ignore_max_compaction_bytes_for_input),
OptionType::kBoolean, OptionVerificationType::kNormal,
{0, OptionType::kBoolean, OptionVerificationType::kDeprecated,
OptionTypeFlags::kMutable}},
{"expanded_compaction_factor",
{0, OptionType::kInt, OptionVerificationType::kDeprecated,
@ -1076,8 +1074,6 @@ void MutableCFOptions::Dump(Logger* log) const {
level0_stop_writes_trigger);
ROCKS_LOG_INFO(log, " max_compaction_bytes: %" PRIu64,
max_compaction_bytes);
ROCKS_LOG_INFO(log, " ignore_max_compaction_bytes_for_input: %s",
ignore_max_compaction_bytes_for_input ? "true" : "false");
ROCKS_LOG_INFO(log, " target_file_size_base: %" PRIu64,
target_file_size_base);
ROCKS_LOG_INFO(log, " target_file_size_multiplier: %d",

View File

@ -132,8 +132,6 @@ struct MutableCFOptions {
level0_slowdown_writes_trigger(options.level0_slowdown_writes_trigger),
level0_stop_writes_trigger(options.level0_stop_writes_trigger),
max_compaction_bytes(options.max_compaction_bytes),
ignore_max_compaction_bytes_for_input(
options.ignore_max_compaction_bytes_for_input),
target_file_size_base(options.target_file_size_base),
target_file_size_multiplier(options.target_file_size_multiplier),
max_bytes_for_level_base(options.max_bytes_for_level_base),
@ -198,7 +196,6 @@ struct MutableCFOptions {
level0_slowdown_writes_trigger(0),
level0_stop_writes_trigger(0),
max_compaction_bytes(0),
ignore_max_compaction_bytes_for_input(true),
target_file_size_base(0),
target_file_size_multiplier(0),
max_bytes_for_level_base(0),
@ -281,7 +278,6 @@ struct MutableCFOptions {
int level0_slowdown_writes_trigger;
int level0_stop_writes_trigger;
uint64_t max_compaction_bytes;
bool ignore_max_compaction_bytes_for_input;
uint64_t target_file_size_base;
int target_file_size_multiplier;
uint64_t max_bytes_for_level_base;

View File

@ -71,8 +71,6 @@ AdvancedColumnFamilyOptions::AdvancedColumnFamilyOptions(const Options& options)
max_bytes_for_level_multiplier_additional(
options.max_bytes_for_level_multiplier_additional),
max_compaction_bytes(options.max_compaction_bytes),
ignore_max_compaction_bytes_for_input(
options.ignore_max_compaction_bytes_for_input),
soft_pending_compaction_bytes_limit(
options.soft_pending_compaction_bytes_limit),
hard_pending_compaction_bytes_limit(
@ -290,8 +288,6 @@ void ColumnFamilyOptions::Dump(Logger* log) const {
ROCKS_LOG_HEADER(
log, " Options.max_compaction_bytes: %" PRIu64,
max_compaction_bytes);
ROCKS_LOG_HEADER(log, " Options.ignore_max_compaction_bytes_for_input: %s",
ignore_max_compaction_bytes_for_input ? "true" : "false");
ROCKS_LOG_HEADER(
log,
" Options.arena_block_size: %" ROCKSDB_PRIszt,

View File

@ -229,8 +229,6 @@ void UpdateColumnFamilyOptions(const MutableCFOptions& moptions,
moptions.level0_slowdown_writes_trigger;
cf_opts->level0_stop_writes_trigger = moptions.level0_stop_writes_trigger;
cf_opts->max_compaction_bytes = moptions.max_compaction_bytes;
cf_opts->ignore_max_compaction_bytes_for_input =
moptions.ignore_max_compaction_bytes_for_input;
cf_opts->target_file_size_base = moptions.target_file_size_base;
cf_opts->target_file_size_multiplier = moptions.target_file_size_multiplier;
cf_opts->max_bytes_for_level_base = moptions.max_bytes_for_level_base;

View File

@ -0,0 +1 @@
Removed deprecated option `ColumnFamilyOptions::ignore_max_compaction_bytes_for_input`