From e7ffca94937f8eb7569c329fc7ad96428e939b13 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Mon, 4 Nov 2024 16:15:10 -0800 Subject: [PATCH] Refactoring toward making preserve/preclude options mutable (#13114) Summary: Move them to MutableCFOptions and perform appropriate refactorings to make that work. I didn't want to mix up refactoring with interesting functional changes. Potentially non-trivial bits here: * During DB Open or RegisterRecordSeqnoTimeWorker we use `GetLatestMutableCFOptions()` because either (a) there might not be a current version, or (b) we are in the process of applying the desired next options. * Upgrade some test infrastructure to allow some options in MutableCFOptions to be mutable (should be a temporary state) * Fix a warning that showed up about uninitialized `paranoid_memory_checks` Pull Request resolved: https://github.com/facebook/rocksdb/pull/13114 Test Plan: existing tests, manually check options are still not settable with SetOptions Reviewed By: jowlyzhang Differential Revision: D65429031 Pulled By: pdillinger fbshipit-source-id: 6e0906d08dd8ddf62731cefffe9b8d94149942b9 --- db/compaction/compaction.cc | 8 ++-- db/compaction/compaction.h | 9 +++-- db/compaction/compaction_job.cc | 8 ++-- db/compaction/compaction_picker.cc | 33 +++++++++------- db/compaction/compaction_picker.h | 5 +-- db/compaction/compaction_picker_level.cc | 15 ++++--- db/compaction/compaction_picker_test.cc | 33 ++++++++-------- db/compaction/compaction_picker_universal.cc | 35 +++++++++-------- db/db_impl/db_impl.cc | 19 +++++---- db/db_impl/db_impl_compaction_flush.cc | 7 +--- db/db_impl/db_impl_open.cc | 5 ++- db/db_options_test.cc | 5 +++ db/flush_job.cc | 6 +-- db/version_set.cc | 4 +- options/cf_options.cc | 41 ++++++++++++++------ options/cf_options.h | 16 ++++++-- options/options_helper.cc | 8 ++-- 17 files changed, 151 insertions(+), 106 deletions(-) diff --git a/db/compaction/compaction.cc b/db/compaction/compaction.cc index 2362cc4170..8444e87c95 100644 --- a/db/compaction/compaction.cc +++ b/db/compaction/compaction.cc @@ -345,8 +345,9 @@ Compaction::Compaction( _compaction_reason == CompactionReason::kExternalSstIngestion || _compaction_reason == CompactionReason::kRefitLevel ? Compaction::kInvalidLevel - : EvaluatePenultimateLevel(vstorage, immutable_options_, - start_level_, output_level_)) { + : EvaluatePenultimateLevel(vstorage, mutable_cf_options_, + immutable_options_, start_level_, + output_level_)) { MarkFilesBeingCompacted(true); if (is_manual_compaction_) { compaction_reason_ = CompactionReason::kManualCompaction; @@ -989,6 +990,7 @@ uint64_t Compaction::MinInputFileEpochNumber() const { int Compaction::EvaluatePenultimateLevel( const VersionStorageInfo* vstorage, + const MutableCFOptions& mutable_cf_options, const ImmutableOptions& immutable_options, const int start_level, const int output_level) { // TODO: currently per_key_placement feature only support level and universal @@ -1020,7 +1022,7 @@ int Compaction::EvaluatePenultimateLevel( } bool supports_per_key_placement = - immutable_options.preclude_last_level_data_seconds > 0; + mutable_cf_options.preclude_last_level_data_seconds > 0; // it could be overridden by unittest TEST_SYNC_POINT_CALLBACK("Compaction::SupportsPerKeyPlacement:Enabled", diff --git a/db/compaction/compaction.h b/db/compaction/compaction.h index f43dc620d8..9e045fb58f 100644 --- a/db/compaction/compaction.h +++ b/db/compaction/compaction.h @@ -440,10 +440,11 @@ class Compaction { // penultimate level. The safe key range is populated by // `PopulatePenultimateLevelOutputRange()`. // Which could potentially disable all penultimate level output. - static int EvaluatePenultimateLevel(const VersionStorageInfo* vstorage, - const ImmutableOptions& immutable_options, - const int start_level, - const int output_level); + static int EvaluatePenultimateLevel( + const VersionStorageInfo* vstorage, + const MutableCFOptions& mutable_cf_options, + const ImmutableOptions& immutable_options, const int start_level, + const int output_level); // mark (or clear) all files that are being compacted void MarkFilesBeingCompacted(bool being_compacted) const; diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index c1e2035d26..cc31c5d595 100644 --- a/db/compaction/compaction_job.cc +++ b/db/compaction/compaction_job.cc @@ -288,8 +288,8 @@ void CompactionJob::Prepare() { // to encode seqno->time to the output files. uint64_t preserve_time_duration = - std::max(c->immutable_options()->preserve_internal_time_seconds, - c->immutable_options()->preclude_last_level_data_seconds); + std::max(c->mutable_cf_options()->preserve_internal_time_seconds, + c->mutable_cf_options()->preclude_last_level_data_seconds); if (preserve_time_duration > 0) { const ReadOptions read_options(Env::IOActivity::kCompaction); @@ -326,8 +326,8 @@ void CompactionJob::Prepare() { seqno_to_time_mapping_.Enforce(_current_time); seqno_to_time_mapping_.GetCurrentTieringCutoffSeqnos( static_cast(_current_time), - c->immutable_options()->preserve_internal_time_seconds, - c->immutable_options()->preclude_last_level_data_seconds, + c->mutable_cf_options()->preserve_internal_time_seconds, + c->mutable_cf_options()->preclude_last_level_data_seconds, &preserve_time_min_seqno_, &preclude_last_level_min_seqno_); } // For accuracy of the GetProximalSeqnoBeforeTime queries above, we only diff --git a/db/compaction/compaction_picker.cc b/db/compaction/compaction_picker.cc index fe1dbcdb9e..eb36225a9c 100644 --- a/db/compaction/compaction_picker.cc +++ b/db/compaction/compaction_picker.cc @@ -351,11 +351,11 @@ Compaction* CompactionPicker::CompactFiles( break; } } - assert(output_level == 0 || - !FilesRangeOverlapWithCompaction( - input_files, output_level, - Compaction::EvaluatePenultimateLevel(vstorage, ioptions_, - start_level, output_level))); + assert(output_level == 0 || !FilesRangeOverlapWithCompaction( + input_files, output_level, + Compaction::EvaluatePenultimateLevel( + vstorage, mutable_cf_options, ioptions_, + start_level, output_level))); #endif /* !NDEBUG */ CompressionType compression_type; @@ -659,8 +659,9 @@ Compaction* CompactionPicker::CompactRange( // overlaping outputs in the same level. if (FilesRangeOverlapWithCompaction( inputs, output_level, - Compaction::EvaluatePenultimateLevel(vstorage, ioptions_, - start_level, output_level))) { + Compaction::EvaluatePenultimateLevel(vstorage, mutable_cf_options, + ioptions_, start_level, + output_level))) { // This compaction output could potentially conflict with the output // of a currently running compaction, we cannot run it. *manual_conflict = true; @@ -846,7 +847,8 @@ Compaction* CompactionPicker::CompactRange( // overlaping outputs in the same level. if (FilesRangeOverlapWithCompaction( compaction_inputs, output_level, - Compaction::EvaluatePenultimateLevel(vstorage, ioptions_, input_level, + Compaction::EvaluatePenultimateLevel(vstorage, mutable_cf_options, + ioptions_, input_level, output_level))) { // This compaction output could potentially conflict with the output // of a currently running compaction, we cannot run it. @@ -1049,10 +1051,12 @@ Status CompactionPicker::SanitizeCompactionInputFilesForAllLevels( } Status CompactionPicker::SanitizeAndConvertCompactionInputFiles( - std::unordered_set* input_files, - const ColumnFamilyMetaData& cf_meta, const int output_level, - const VersionStorageInfo* vstorage, + std::unordered_set* input_files, const int output_level, + Version* version, std::vector* converted_input_files) const { + ColumnFamilyMetaData cf_meta; + version->GetColumnFamilyMetaData(&cf_meta); + assert(static_cast(cf_meta.levels.size()) - 1 == cf_meta.levels[cf_meta.levels.size() - 1].level); assert(converted_input_files); @@ -1123,7 +1127,8 @@ Status CompactionPicker::SanitizeAndConvertCompactionInputFiles( } s = GetCompactionInputsFromFileNumbers(converted_input_files, input_files, - vstorage, CompactionOptions()); + version->storage_info(), + CompactionOptions()); if (!s.ok()) { return s; } @@ -1132,8 +1137,8 @@ Status CompactionPicker::SanitizeAndConvertCompactionInputFiles( FilesRangeOverlapWithCompaction( *converted_input_files, output_level, Compaction::EvaluatePenultimateLevel( - vstorage, ioptions_, (*converted_input_files)[0].level, - output_level))) { + version->storage_info(), version->GetMutableCFOptions(), + ioptions_, (*converted_input_files)[0].level, output_level))) { return Status::Aborted( "A running compaction is writing to the same output level(s) in an " "overlapping key range"); diff --git a/db/compaction/compaction_picker.h b/db/compaction/compaction_picker.h index 6729cda0ae..33e6b38e5b 100644 --- a/db/compaction/compaction_picker.h +++ b/db/compaction/compaction_picker.h @@ -97,9 +97,8 @@ class CompactionPicker { // non-ok status with specific reason. // Status SanitizeAndConvertCompactionInputFiles( - std::unordered_set* input_files, - const ColumnFamilyMetaData& cf_meta, const int output_level, - const VersionStorageInfo* vstorage, + std::unordered_set* input_files, const int output_level, + Version* version, std::vector* converted_input_files) const; // Free up the files that participated in a compaction diff --git a/db/compaction/compaction_picker_level.cc b/db/compaction/compaction_picker_level.cc index 262ebba85e..612c1e5af2 100644 --- a/db/compaction/compaction_picker_level.cc +++ b/db/compaction/compaction_picker_level.cc @@ -414,8 +414,9 @@ void LevelCompactionBuilder::SetupOtherFilesWithRoundRobinExpansion() { &tmp_start_level_inputs) || compaction_picker_->FilesRangeOverlapWithCompaction( {tmp_start_level_inputs}, output_level_, - Compaction::EvaluatePenultimateLevel( - vstorage_, ioptions_, start_level_, output_level_))) { + Compaction::EvaluatePenultimateLevel(vstorage_, mutable_cf_options_, + ioptions_, start_level_, + output_level_))) { // Constraint 1a tmp_start_level_inputs.clear(); return; @@ -489,8 +490,9 @@ bool LevelCompactionBuilder::SetupOtherInputsIfNeeded() { // We need to disallow this from happening. if (compaction_picker_->FilesRangeOverlapWithCompaction( compaction_inputs_, output_level_, - Compaction::EvaluatePenultimateLevel( - vstorage_, ioptions_, start_level_, output_level_))) { + Compaction::EvaluatePenultimateLevel(vstorage_, mutable_cf_options_, + ioptions_, start_level_, + output_level_))) { // This compaction output could potentially conflict with the output // of a currently running compaction, we cannot run it. return false; @@ -844,8 +846,9 @@ bool LevelCompactionBuilder::PickFileToCompact() { &start_level_inputs_) || compaction_picker_->FilesRangeOverlapWithCompaction( {start_level_inputs_}, output_level_, - Compaction::EvaluatePenultimateLevel( - vstorage_, ioptions_, start_level_, output_level_))) { + Compaction::EvaluatePenultimateLevel(vstorage_, mutable_cf_options_, + ioptions_, start_level_, + output_level_))) { // A locked (pending compaction) input-level file was pulled in due to // user-key overlap. start_level_inputs_.clear(); diff --git a/db/compaction/compaction_picker_test.cc b/db/compaction/compaction_picker_test.cc index b75df61c3b..3b9ddd9034 100644 --- a/db/compaction/compaction_picker_test.cc +++ b/db/compaction/compaction_picker_test.cc @@ -3609,7 +3609,7 @@ TEST_F(CompactionPickerTest, UniversalSizeAmpTierCompactionNonLastLevel) { const int kLastLevel = kNumLevels - 1; ioptions_.compaction_style = kCompactionStyleUniversal; - ioptions_.preclude_last_level_data_seconds = 1000; + mutable_cf_options_.preclude_last_level_data_seconds = 1000; mutable_cf_options_.compaction_options_universal .max_size_amplification_percent = 200; // To avoid any L0 file exclusion in size amp compaction intended for reducing @@ -3649,7 +3649,7 @@ TEST_F(CompactionPickerTest, UniversalSizeRatioTierCompactionLastLevel) { const int kPenultimateLevel = kLastLevel - 1; ioptions_.compaction_style = kCompactionStyleUniversal; - ioptions_.preclude_last_level_data_seconds = 1000; + mutable_cf_options_.preclude_last_level_data_seconds = 1000; mutable_cf_options_.compaction_options_universal .max_size_amplification_percent = 200; UniversalCompactionPicker universal_compaction_picker(ioptions_, &icmp_); @@ -3687,7 +3687,7 @@ TEST_F(CompactionPickerTest, UniversalSizeAmpTierCompactionNotSuport) { const int kLastLevel = kNumLevels - 1; ioptions_.compaction_style = kCompactionStyleUniversal; - ioptions_.preclude_last_level_data_seconds = 1000; + mutable_cf_options_.preclude_last_level_data_seconds = 1000; mutable_cf_options_.compaction_options_universal .max_size_amplification_percent = 200; // To avoid any L0 file exclusion in size amp compaction intended for reducing @@ -3725,7 +3725,7 @@ TEST_F(CompactionPickerTest, UniversalSizeAmpTierCompactionLastLevel) { const int kPenultimateLevel = kLastLevel - 1; ioptions_.compaction_style = kCompactionStyleUniversal; - ioptions_.preclude_last_level_data_seconds = 1000; + mutable_cf_options_.preclude_last_level_data_seconds = 1000; mutable_cf_options_.compaction_options_universal .max_size_amplification_percent = 200; UniversalCompactionPicker universal_compaction_picker(ioptions_, &icmp_); @@ -3909,8 +3909,8 @@ TEST_P(PerKeyPlacementCompactionPickerTest, OverlapWithNormalCompaction) { ASSERT_EQ(enable_per_key_placement_, level_compaction_picker.FilesRangeOverlapWithCompaction( input_files, 6, - Compaction::EvaluatePenultimateLevel(vstorage_.get(), ioptions_, - 0, 6))); + Compaction::EvaluatePenultimateLevel( + vstorage_.get(), mutable_cf_options_, ioptions_, 0, 6))); } TEST_P(PerKeyPlacementCompactionPickerTest, NormalCompactionOverlap) { @@ -3997,8 +3997,8 @@ TEST_P(PerKeyPlacementCompactionPickerTest, ASSERT_EQ(enable_per_key_placement_, universal_compaction_picker.FilesRangeOverlapWithCompaction( input_files, 6, - Compaction::EvaluatePenultimateLevel(vstorage_.get(), ioptions_, - 0, 6))); + Compaction::EvaluatePenultimateLevel( + vstorage_.get(), mutable_cf_options_, ioptions_, 0, 6))); } TEST_P(PerKeyPlacementCompactionPickerTest, NormalCompactionOverlapUniversal) { @@ -4049,7 +4049,7 @@ TEST_P(PerKeyPlacementCompactionPickerTest, PenultimateOverlapUniversal) { // This test is make sure the Tiered compaction would lock whole range of // both output level and penultimate level if (enable_per_key_placement_) { - ioptions_.preclude_last_level_data_seconds = 10000; + mutable_cf_options_.preclude_last_level_data_seconds = 10000; } int num_levels = ioptions_.num_levels; @@ -4104,7 +4104,7 @@ TEST_P(PerKeyPlacementCompactionPickerTest, PenultimateOverlapUniversal) { TEST_P(PerKeyPlacementCompactionPickerTest, LastLevelOnlyOverlapUniversal) { if (enable_per_key_placement_) { - ioptions_.preclude_last_level_data_seconds = 10000; + mutable_cf_options_.preclude_last_level_data_seconds = 10000; } int num_levels = ioptions_.num_levels; @@ -4163,7 +4163,7 @@ TEST_P(PerKeyPlacementCompactionPickerTest, // This should rarely happen in universal compaction, as the non-empty L5 // should be included in the compaction. if (enable_per_key_placement_) { - ioptions_.preclude_last_level_data_seconds = 10000; + mutable_cf_options_.preclude_last_level_data_seconds = 10000; } int num_levels = ioptions_.num_levels; @@ -4217,7 +4217,7 @@ TEST_P(PerKeyPlacementCompactionPickerTest, // penultimate level compaction if there's already an ongoing compaction to // the penultimate level if (enable_per_key_placement_) { - ioptions_.preclude_last_level_data_seconds = 10000; + mutable_cf_options_.preclude_last_level_data_seconds = 10000; } int num_levels = ioptions_.num_levels; @@ -4258,8 +4258,8 @@ TEST_P(PerKeyPlacementCompactionPickerTest, ASSERT_EQ(enable_per_key_placement_, universal_compaction_picker.FilesRangeOverlapWithCompaction( input_files, 6, - Compaction::EvaluatePenultimateLevel(vstorage_.get(), ioptions_, - 6, 6))); + Compaction::EvaluatePenultimateLevel( + vstorage_.get(), mutable_cf_options_, ioptions_, 6, 6))); if (!enable_per_key_placement_) { std::unique_ptr comp2(universal_compaction_picker.CompactFiles( @@ -4277,7 +4277,7 @@ TEST_P(PerKeyPlacementCompactionPickerTest, // compaction, so it's safe to move data from the last level to the // penultimate level. if (enable_per_key_placement_) { - ioptions_.preclude_last_level_data_seconds = 10000; + mutable_cf_options_.preclude_last_level_data_seconds = 10000; } int num_levels = ioptions_.num_levels; @@ -4318,7 +4318,8 @@ TEST_P(PerKeyPlacementCompactionPickerTest, // always safe to move data up ASSERT_FALSE(universal_compaction_picker.FilesRangeOverlapWithCompaction( input_files, 6, - Compaction::EvaluatePenultimateLevel(vstorage_.get(), ioptions_, 6, 6))); + Compaction::EvaluatePenultimateLevel(vstorage_.get(), mutable_cf_options_, + ioptions_, 6, 6))); // 2 compactions can be run in parallel std::unique_ptr comp2(universal_compaction_picker.CompactFiles( diff --git a/db/compaction/compaction_picker_universal.cc b/db/compaction/compaction_picker_universal.cc index c1ad96bd88..427abb9eab 100644 --- a/db/compaction/compaction_picker_universal.cc +++ b/db/compaction/compaction_picker_universal.cc @@ -142,7 +142,7 @@ class UniversalCompactionBuilder { bool ShouldSkipLastSortedRunForSizeAmpCompaction() const { assert(!sorted_runs_.empty()); - return ioptions_.preclude_last_level_data_seconds > 0 && + return mutable_cf_options_.preclude_last_level_data_seconds > 0 && ioptions_.num_levels > 2 && sorted_runs_.back().level == ioptions_.num_levels - 1 && sorted_runs_.size() > 1; @@ -994,11 +994,11 @@ Compaction* UniversalCompactionBuilder::PickCompactionToReduceSortedRuns( grandparents = vstorage_->LevelFiles(sorted_runs_[first_index_after].level); } - if (output_level != 0 && - picker_->FilesRangeOverlapWithCompaction( - inputs, output_level, - Compaction::EvaluatePenultimateLevel(vstorage_, ioptions_, - start_level, output_level))) { + if (output_level != 0 && picker_->FilesRangeOverlapWithCompaction( + inputs, output_level, + Compaction::EvaluatePenultimateLevel( + vstorage_, mutable_cf_options_, ioptions_, + start_level, output_level))) { return nullptr; } CompactionReason compaction_reason; @@ -1343,11 +1343,11 @@ Compaction* UniversalCompactionBuilder::PickIncrementalForReduceSizeAmp( } // intra L0 compactions outputs could have overlap - if (output_level != 0 && - picker_->FilesRangeOverlapWithCompaction( - inputs, output_level, - Compaction::EvaluatePenultimateLevel(vstorage_, ioptions_, - start_level, output_level))) { + if (output_level != 0 && picker_->FilesRangeOverlapWithCompaction( + inputs, output_level, + Compaction::EvaluatePenultimateLevel( + vstorage_, mutable_cf_options_, ioptions_, + start_level, output_level))) { return nullptr; } @@ -1487,7 +1487,8 @@ Compaction* UniversalCompactionBuilder::PickDeleteTriggeredCompaction() { if (picker_->FilesRangeOverlapWithCompaction( inputs, output_level, Compaction::EvaluatePenultimateLevel( - vstorage_, ioptions_, start_level, output_level))) { + vstorage_, mutable_cf_options_, ioptions_, start_level, + output_level))) { return nullptr; } @@ -1587,11 +1588,11 @@ Compaction* UniversalCompactionBuilder::PickCompactionWithSortedRunRange( } // intra L0 compactions outputs could have overlap - if (output_level != 0 && - picker_->FilesRangeOverlapWithCompaction( - inputs, output_level, - Compaction::EvaluatePenultimateLevel(vstorage_, ioptions_, - start_level, output_level))) { + if (output_level != 0 && picker_->FilesRangeOverlapWithCompaction( + inputs, output_level, + Compaction::EvaluatePenultimateLevel( + vstorage_, mutable_cf_options_, ioptions_, + start_level, output_level))) { return nullptr; } diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index a3061a3c61..3fabfabe31 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -852,10 +852,11 @@ Status DBImpl::RegisterRecordSeqnoTimeWorker(const ReadOptions& read_options, InstrumentedMutexLock l(&mutex_); for (auto cfd : *versions_->GetColumnFamilySet()) { + auto& mopts = *cfd->GetLatestMutableCFOptions(); // preserve time is the max of 2 options. uint64_t preserve_seconds = - std::max(cfd->ioptions()->preserve_internal_time_seconds, - cfd->ioptions()->preclude_last_level_data_seconds); + std::max(mopts.preserve_internal_time_seconds, + mopts.preclude_last_level_data_seconds); if (!cfd->IsDropped() && preserve_seconds > 0) { min_preserve_seconds = std::min(preserve_seconds, min_preserve_seconds); max_preserve_seconds = std::max(preserve_seconds, max_preserve_seconds); @@ -3719,6 +3720,9 @@ Status DBImpl::DropColumnFamilyImpl(ColumnFamilyHandle* column_family) { edit.SetColumnFamily(cfd->GetID()); Status s; + // Save re-aquiring lock for RegisterRecordSeqnoTimeWorker when not + // applicable + bool used_preserve_preclude = false; { InstrumentedMutexLock l(&mutex_); if (cfd->IsDropped()) { @@ -3734,9 +3738,11 @@ Status DBImpl::DropColumnFamilyImpl(ColumnFamilyHandle* column_family) { write_thread_.ExitUnbatched(&w); } if (s.ok()) { - auto* mutable_cf_options = cfd->GetLatestMutableCFOptions(); - max_total_in_memory_state_ -= mutable_cf_options->write_buffer_size * - mutable_cf_options->max_write_buffer_number; + auto& moptions = *cfd->GetLatestMutableCFOptions(); + max_total_in_memory_state_ -= + moptions.write_buffer_size * moptions.max_write_buffer_number; + used_preserve_preclude = moptions.preserve_internal_time_seconds > 0 || + moptions.preclude_last_level_data_seconds > 0; } if (!cf_support_snapshot) { @@ -3754,8 +3760,7 @@ Status DBImpl::DropColumnFamilyImpl(ColumnFamilyHandle* column_family) { bg_cv_.SignalAll(); } - if (cfd->ioptions()->preserve_internal_time_seconds > 0 || - cfd->ioptions()->preclude_last_level_data_seconds > 0) { + if (used_preserve_preclude) { s = RegisterRecordSeqnoTimeWorker(read_options, write_options, /* is_new_db */ false); } diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index 12de06d1d6..0fa85bb2dd 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -1457,11 +1457,6 @@ Status DBImpl::CompactFilesImpl( input_set.insert(TableFileNameToNumber(file_name)); } - ColumnFamilyMetaData cf_meta; - // TODO(yhchiang): can directly use version here if none of the - // following functions call is pluggable to external developers. - version->GetColumnFamilyMetaData(&cf_meta); - if (output_path_id < 0) { if (cfd->ioptions()->cf_paths.size() == 1U) { output_path_id = 0; @@ -1482,7 +1477,7 @@ Status DBImpl::CompactFilesImpl( std::vector input_files; Status s = cfd->compaction_picker()->SanitizeAndConvertCompactionInputFiles( - &input_set, cf_meta, output_level, version->storage_info(), &input_files); + &input_set, output_level, version, &input_files); TEST_SYNC_POINT( "DBImpl::CompactFilesImpl::PostSanitizeAndConvertCompactionInputFiles"); if (!s.ok()) { diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 8586758f28..7e5f08897a 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -575,6 +575,7 @@ Status DBImpl::Recover( } if (s.ok() && !read_only) { for (auto cfd : *versions_->GetColumnFamilySet()) { + auto& moptions = *cfd->GetLatestMutableCFOptions(); // Try to trivially move files down the LSM tree to start from bottommost // level when level_compaction_dynamic_level_bytes is enabled. This should // only be useful when user is migrating to turning on this option. @@ -592,14 +593,14 @@ Status DBImpl::Recover( if (cfd->ioptions()->compaction_style == CompactionStyle::kCompactionStyleLevel && cfd->ioptions()->level_compaction_dynamic_level_bytes && - !cfd->GetLatestMutableCFOptions()->disable_auto_compactions) { + !moptions.disable_auto_compactions) { int to_level = cfd->ioptions()->num_levels - 1; // last level is reserved // allow_ingest_behind does not support Level Compaction, // and per_key_placement can have infinite compaction loop for Level // Compaction. Adjust to_level here just to be safe. if (cfd->ioptions()->allow_ingest_behind || - cfd->ioptions()->preclude_last_level_data_seconds > 0) { + moptions.preclude_last_level_data_seconds > 0) { to_level -= 1; } // Whether this column family has a level trivially moved diff --git a/db/db_options_test.cc b/db/db_options_test.cc index e2e96e59f3..5785b429c5 100644 --- a/db/db_options_test.cc +++ b/db/db_options_test.cc @@ -56,6 +56,11 @@ class DBOptionsTest : public DBTestBase { EXPECT_OK(GetStringFromMutableCFOptions( config_options, MutableCFOptions(options), &options_str)); EXPECT_OK(StringToMap(options_str, &mutable_map)); + for (auto& opt : TEST_GetImmutableInMutableCFOptions()) { + // Not yet mutable but migrated to MutableCFOptions in preparation for + // being mutable + mutable_map.erase(opt); + } return mutable_map; } diff --git a/db/flush_job.cc b/db/flush_job.cc index 56f5170c69..69ce103022 100644 --- a/db/flush_job.cc +++ b/db/flush_job.cc @@ -1191,7 +1191,7 @@ void FlushJob::GetEffectiveCutoffUDTForPickedMemTables() { } void FlushJob::GetPrecludeLastLevelMinSeqno() { - if (cfd_->ioptions()->preclude_last_level_data_seconds == 0) { + if (mutable_cf_options_.preclude_last_level_data_seconds == 0) { return; } int64_t current_time = 0; @@ -1204,8 +1204,8 @@ void FlushJob::GetPrecludeLastLevelMinSeqno() { SequenceNumber preserve_time_min_seqno; seqno_to_time_mapping_->GetCurrentTieringCutoffSeqnos( static_cast(current_time), - cfd_->ioptions()->preserve_internal_time_seconds, - cfd_->ioptions()->preclude_last_level_data_seconds, + mutable_cf_options_.preserve_internal_time_seconds, + mutable_cf_options_.preclude_last_level_data_seconds, &preserve_time_min_seqno, &preclude_last_level_min_seqno_); } } diff --git a/db/version_set.cc b/db/version_set.cc index a7bbd5af6d..aa2a49a1b7 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -4757,7 +4757,7 @@ void VersionStorageInfo::CalculateBaseBytes(const ImmutableOptions& ioptions, cur_level_size / options.max_bytes_for_level_multiplier); if (lowest_unnecessary_level_ == -1 && cur_level_size <= base_bytes_min && - (ioptions.preclude_last_level_data_seconds == 0 || + (options.preclude_last_level_data_seconds == 0 || i < num_levels_ - 2)) { // When per_key_placement is enabled, the penultimate level is // necessary. @@ -4773,7 +4773,7 @@ void VersionStorageInfo::CalculateBaseBytes(const ImmutableOptions& ioptions, // which can less than base_bytes_min AND necessary, // or there is some unnecessary level. assert(first_non_empty_level == num_levels_ - 1 || - ioptions.preclude_last_level_data_seconds > 0 || + options.preclude_last_level_data_seconds > 0 || lowest_unnecessary_level_ != -1); // Case 1. If we make target size of last level to be max_level_size, // target size of the first non-empty level would be smaller than diff --git a/options/cf_options.cc b/options/cf_options.cc index 8c5751cee6..04a42fa860 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -551,6 +552,14 @@ static std::unordered_map {offsetof(struct MutableCFOptions, periodic_compaction_seconds), OptionType::kUInt64T, OptionVerificationType::kNormal, OptionTypeFlags::kMutable}}, + {"preclude_last_level_data_seconds", + {offsetof(struct MutableCFOptions, preclude_last_level_data_seconds), + OptionType::kUInt64T, OptionVerificationType::kNormal, + OptionTypeFlags::kNone}}, + {"preserve_internal_time_seconds", + {offsetof(struct MutableCFOptions, preserve_internal_time_seconds), + OptionType::kUInt64T, OptionVerificationType::kNormal, + OptionTypeFlags::kNone}}, {"bottommost_temperature", {0, OptionType::kTemperature, OptionVerificationType::kDeprecated, OptionTypeFlags::kMutable}}, @@ -731,14 +740,6 @@ static std::unordered_map {offsetof(struct ImmutableCFOptions, default_temperature), OptionType::kTemperature, OptionVerificationType::kNormal, OptionTypeFlags::kCompareNever}}, - {"preclude_last_level_data_seconds", - {offsetof(struct ImmutableCFOptions, preclude_last_level_data_seconds), - OptionType::kUInt64T, OptionVerificationType::kNormal, - OptionTypeFlags::kNone}}, - {"preserve_internal_time_seconds", - {offsetof(struct ImmutableCFOptions, preserve_internal_time_seconds), - OptionType::kUInt64T, OptionVerificationType::kNormal, - OptionTypeFlags::kNone}}, // Need to keep this around to be able to read old OPTIONS files. {"max_mem_compaction_level", {0, OptionType::kInt, OptionVerificationType::kDeprecated, @@ -998,9 +999,6 @@ ImmutableCFOptions::ImmutableCFOptions(const ColumnFamilyOptions& cf_options) optimize_filters_for_hits(cf_options.optimize_filters_for_hits), force_consistency_checks(cf_options.force_consistency_checks), default_temperature(cf_options.default_temperature), - preclude_last_level_data_seconds( - cf_options.preclude_last_level_data_seconds), - preserve_internal_time_seconds(cf_options.preserve_internal_time_seconds), memtable_insert_with_hint_prefix_extractor( cf_options.memtable_insert_with_hint_prefix_extractor), cf_paths(cf_options.cf_paths), @@ -1142,6 +1140,11 @@ void MutableCFOptions::Dump(Logger* log) const { ttl); ROCKS_LOG_INFO(log, " periodic_compaction_seconds: %" PRIu64, periodic_compaction_seconds); + ROCKS_LOG_INFO(log, + " preclude_last_level_data_seconds: %" PRIu64, + preclude_last_level_data_seconds); + ROCKS_LOG_INFO(log, " preserve_internal_time_seconds: %" PRIu64, + preserve_internal_time_seconds); ROCKS_LOG_INFO(log, " paranoid_memory_checks: %d", paranoid_memory_checks); std::string result; @@ -1255,4 +1258,20 @@ Status GetStringFromMutableCFOptions(const ConfigOptions& config_options, return OptionTypeInfo::SerializeType( config_options, cf_mutable_options_type_info, &mutable_opts, opt_string); } + +#ifndef NDEBUG +std::vector TEST_GetImmutableInMutableCFOptions() { + std::vector result; + for (const auto& opt : cf_mutable_options_type_info) { + if (!opt.second.IsMutable()) { + result.emplace_back(opt.first); + } + } + if (result.size() > 0) { + std::cerr << "Warning: " << result.size() << " immutable options in " + << "MutableCFOptions" << std::endl; + } + return result; +} +#endif // !NDEBUG } // namespace ROCKSDB_NAMESPACE diff --git a/options/cf_options.h b/options/cf_options.h index 5cc46712ce..a71343629e 100644 --- a/options/cf_options.h +++ b/options/cf_options.h @@ -70,10 +70,6 @@ struct ImmutableCFOptions { Temperature default_temperature; - uint64_t preclude_last_level_data_seconds; - - uint64_t preserve_internal_time_seconds; - std::shared_ptr memtable_insert_with_hint_prefix_extractor; @@ -142,6 +138,9 @@ struct MutableCFOptions { options.max_bytes_for_level_multiplier_additional), compaction_options_fifo(options.compaction_options_fifo), compaction_options_universal(options.compaction_options_universal), + preclude_last_level_data_seconds( + options.preclude_last_level_data_seconds), + preserve_internal_time_seconds(options.preserve_internal_time_seconds), enable_blob_files(options.enable_blob_files), min_blob_size(options.min_blob_size), blob_file_size(options.blob_file_size), @@ -204,6 +203,8 @@ struct MutableCFOptions { ttl(0), periodic_compaction_seconds(0), compaction_options_fifo(), + preclude_last_level_data_seconds(0), + preserve_internal_time_seconds(0), enable_blob_files(false), min_blob_size(0), blob_file_size(0), @@ -223,6 +224,7 @@ struct MutableCFOptions { default_write_temperature(Temperature::kUnknown), memtable_protection_bytes_per_key(0), block_protection_bytes_per_key(0), + paranoid_memory_checks(false), sample_for_compression(0), memtable_max_range_deletions(0), bottommost_file_compaction_delay(0), @@ -296,6 +298,8 @@ struct MutableCFOptions { std::vector max_bytes_for_level_multiplier_additional; CompactionOptionsFIFO compaction_options_fifo; CompactionOptionsUniversal compaction_options_universal; + uint64_t preclude_last_level_data_seconds; + uint64_t preserve_internal_time_seconds; // Blob file related options bool enable_blob_files; @@ -354,4 +358,8 @@ Status GetMutableOptionsFromStrings( const std::unordered_map& options_map, Logger* info_log, MutableCFOptions* new_options); +#ifndef NDEBUG +std::vector TEST_GetImmutableInMutableCFOptions(); +#endif + } // namespace ROCKSDB_NAMESPACE diff --git a/options/options_helper.cc b/options/options_helper.cc index 007aaeaa1d..e18c93435f 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -257,6 +257,10 @@ void UpdateColumnFamilyOptions(const MutableCFOptions& moptions, moptions.max_bytes_for_level_multiplier; cf_opts->ttl = moptions.ttl; cf_opts->periodic_compaction_seconds = moptions.periodic_compaction_seconds; + cf_opts->preclude_last_level_data_seconds = + moptions.preclude_last_level_data_seconds; + cf_opts->preserve_internal_time_seconds = + moptions.preserve_internal_time_seconds; cf_opts->max_bytes_for_level_multiplier_additional.clear(); for (auto value : moptions.max_bytes_for_level_multiplier_additional) { @@ -330,10 +334,6 @@ void UpdateColumnFamilyOptions(const ImmutableCFOptions& ioptions, cf_opts->compaction_thread_limiter = ioptions.compaction_thread_limiter; cf_opts->sst_partitioner_factory = ioptions.sst_partitioner_factory; cf_opts->blob_cache = ioptions.blob_cache; - cf_opts->preclude_last_level_data_seconds = - ioptions.preclude_last_level_data_seconds; - cf_opts->preserve_internal_time_seconds = - ioptions.preserve_internal_time_seconds; cf_opts->persist_user_defined_timestamps = ioptions.persist_user_defined_timestamps; cf_opts->default_temperature = ioptions.default_temperature;