From cdf081eea00e5aa8bcc05759efa6c84ac361cc0d Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 25 Jun 2021 11:16:46 -0400 Subject: [PATCH 01/39] add ImmutableCFOption disable_preload_pinning --- db/version_builder.cc | 16 ++++++- include/rocksdb/advanced_options.h | 9 ++++ include/rocksdb/utilities/ldb_cmd.h | 4 ++ java/rocksjni/options.cc | 47 +++++++++++++++++++ .../AdvancedColumnFamilyOptionsInterface.java | 26 ++++++++++ .../java/org/rocksdb/ColumnFamilyOptions.java | 14 ++++++ java/src/main/java/org/rocksdb/Options.java | 14 ++++++ .../org/rocksdb/ColumnFamilyOptionsTest.java | 10 ++++ .../test/java/org/rocksdb/OptionsTest.java | 10 ++++ options/cf_options.cc | 5 ++ options/cf_options.h | 2 + options/options.cc | 3 ++ options/options_helper.cc | 1 + options/options_settable_test.cc | 1 + options/options_test.cc | 4 ++ test_util/testutil.cc | 1 + tools/ldb_cmd.cc | 6 +++ tools/ldb_tool.cc | 2 + 18 files changed, 173 insertions(+), 2 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 474169bda74..5fd61de16ee 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -922,6 +922,7 @@ class VersionBuilder::Rep { const SliceTransform* prefix_extractor, size_t max_file_size_for_l0_meta_pin) { assert(table_cache_ != nullptr); + assert(false == ioptions_->disable_preload_pinning); size_t table_cache_capacity = table_cache_->get_cache()->GetCapacity(); bool always_load = (table_cache_capacity == TableCache::kInfiniteCapacity); @@ -991,10 +992,21 @@ class VersionBuilder::Rep { true /* record_read_stats */, internal_stats->GetFileReadHist(level), false, level, prefetch_index_and_filter_in_cache, max_file_size_for_l0_meta_pin); + + // The code is attempting two things: + // 1. preload / warm the table cache with new file objects + // 2. create higher performance via a cache lookup avoidance + // The issue is that number 2 creates permanent objects in the + // table cache which over time are no longer useful. The + // disable_preload_pinning option keeps #1 and disables #2. if (file_meta->table_reader_handle != nullptr) { - // Load table_reader - file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( + if (!ioptions_->disable_preload_pinning) { + file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( file_meta->table_reader_handle); + } else { + table_cache_->ReleaseHandle(file_meta->table_reader_handle); + file_meta->table_reader_handle = nullptr; + } } } }); diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index 7804ec46b6c..dce179a1c2a 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -703,6 +703,15 @@ struct AdvancedColumnFamilyOptions { // Default: true bool force_consistency_checks = true; + // RocksDB uses the first 25% of num_open_files for precaching during + // start-up and after compactions. The files precached in this fashion + // provide faster access. However, these files are also never released. + // Scenarios that have large bloom filters not cached or scenarios where + // user is manually lowering the num_open_files at runtime might want + // to disable this behavior. + // Default: false + bool disable_preload_pinning = false; + // Measure IO stats in compactions and flushes, if true. // // Default: false diff --git a/include/rocksdb/utilities/ldb_cmd.h b/include/rocksdb/utilities/ldb_cmd.h index e900abefee5..169f8c50c8a 100644 --- a/include/rocksdb/utilities/ldb_cmd.h +++ b/include/rocksdb/utilities/ldb_cmd.h @@ -61,6 +61,7 @@ class LDBCommand { static const std::string ARG_CREATE_IF_MISSING; static const std::string ARG_NO_VALUE; static const std::string ARG_DISABLE_CONSISTENCY_CHECKS; + static const std::string ARG_DISABLE_PRELOAD_PINNING; struct ParsedParams { std::string cmd; @@ -173,6 +174,9 @@ class LDBCommand { // The value passed to options.force_consistency_checks. bool force_consistency_checks_; + // The value passed to options.disable_preload_pinning. + bool disable_preload_pinning_; + bool create_if_missing_; /** diff --git a/java/rocksjni/options.cc b/java/rocksjni/options.cc index fbf3241792e..1fc9daa2abf 100644 --- a/java/rocksjni/options.cc +++ b/java/rocksjni/options.cc @@ -3663,6 +3663,17 @@ void Java_org_rocksdb_Options_setForceConsistencyChecks( opts->force_consistency_checks = static_cast(jforce_consistency_checks); } +/* + * Class: org_rocksdb_Options + * Method: setDisablePreloadPinning + * Signature: (JZ)V + */ +void Java_org_rocksdb_Options_setDisablePreloadPinning( + JNIEnv*, jobject, jlong jhandle, jboolean jdisable_preload_pinning) { + auto* opts = reinterpret_cast(jhandle); + opts->disable_preload_pinning = static_cast(jdisable_preload_pinning); +} + /* * Class: org_rocksdb_Options * Method: forceConsistencyChecks @@ -3674,6 +3685,17 @@ jboolean Java_org_rocksdb_Options_forceConsistencyChecks( return static_cast(opts->force_consistency_checks); } +/* + * Class: org_rocksdb_Options + * Method: disablePreloadPinning + * Signature: (J)Z + */ +jboolean Java_org_rocksdb_Options_disablePreloadPinning( + JNIEnv*, jobject, jlong jhandle) { + auto* opts = reinterpret_cast(jhandle); + return static_cast(opts->disable_preload_pinning); +} + ////////////////////////////////////////////////////////////////////////////// // ROCKSDB_NAMESPACE::ColumnFamilyOptions @@ -5203,6 +5225,31 @@ jboolean Java_org_rocksdb_ColumnFamilyOptions_forceConsistencyChecks( return static_cast(cf_opts->force_consistency_checks); } +/* + * Class: org_rocksdb_ColumnFamilyOptions + * Method: setDisablePreloadPinning + * Signature: (JZ)V + */ +void Java_org_rocksdb_ColumnFamilyOptions_setDisablePreloadPinning( + JNIEnv*, jobject, jlong jhandle, jboolean jdisable_preload_pinning) { + auto* cf_opts = + reinterpret_cast(jhandle); + cf_opts->disable_preload_pinning = + static_cast(jdisable_preload_pinning); +} + +/* + * Class: org_rocksdb_ColumnFamilyOptions + * Method: disablePreloadPinning + * Signature: (J)Z + */ +jboolean Java_org_rocksdb_ColumnFamilyOptions_disablePreloadPinning( + JNIEnv*, jobject, jlong jhandle) { + auto* cf_opts = + reinterpret_cast(jhandle); + return static_cast(cf_opts->disable_preload_pinning); +} + ///////////////////////////////////////////////////////////////////// // ROCKSDB_NAMESPACE::DBOptions diff --git a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java index 76d9bde4646..4f33843cadf 100644 --- a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java +++ b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java @@ -462,4 +462,30 @@ T setForceConsistencyChecks( * @return true if consistency checks are enforced */ boolean forceConsistencyChecks(); + + /** + * RocksDB uses the first 25% of num_open_files for precaching during + * start-up and after compactions. The files precached in this fashion + * provide faster access. However, these files are also never released. + * Scenarios that have large bloom filters not cached or scenarios where + * user is manually lowering the num_open_files at runtime might want + * to disable this behavior. + * + * Default: false + * + * @param disablePreloadPinning true to stop pinning preload files in cache + * + * @return the reference to the current options. + */ + T setDisablePreloadPinning( + boolean disablePreloadPinning); + + /** + * RocksDB uses the first 25% of num_open_files for precaching during + * start-up and after compactions. The files precached in this fashion + * might or might not be pinned in the table cache. + * + * @return true when pinning preloaded files into cache is disable + */ + boolean DisablePreloadPinning(); } diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java index 72149bf2669..b18a0e151fe 100644 --- a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java +++ b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java @@ -895,6 +895,17 @@ public boolean forceConsistencyChecks() { return forceConsistencyChecks(nativeHandle_); } + @Override + public ColumnFamilyOptions setDisablePreloadPinning(final boolean disablePreloadPinning) { + setDisablePreloadPinning(nativeHandle_, disablePreloadPinning); + return this; + } + + @Override + public boolean disablePreloadPinning() { + return disablePreloadPinning(nativeHandle_); + } + @Override public ColumnFamilyOptions setSstPartitionerFactory(SstPartitionerFactory sstPartitionerFactory) { setSstPartitionerFactory(nativeHandle_, sstPartitionerFactory.nativeHandle_); @@ -1090,6 +1101,9 @@ private native void setCompactionOptionsFIFO(final long handle, private native void setForceConsistencyChecks(final long handle, final boolean forceConsistencyChecks); private native boolean forceConsistencyChecks(final long handle); + private native void setDisablePreloadPinning(final long handle, + final boolean disablePreloadPinning); + private native boolean disablePreloadPinning(final long handle); private native void setSstPartitionerFactory(long nativeHandle_, long newFactoryHandle); private static native void setCompactionThreadLimiter( final long nativeHandle_, final long compactionThreadLimiterHandle); diff --git a/java/src/main/java/org/rocksdb/Options.java b/java/src/main/java/org/rocksdb/Options.java index 57f3aeffbd3..e31017650d2 100644 --- a/java/src/main/java/org/rocksdb/Options.java +++ b/java/src/main/java/org/rocksdb/Options.java @@ -1886,6 +1886,17 @@ public boolean forceConsistencyChecks() { return forceConsistencyChecks(nativeHandle_); } + @Override + public Options setDisablePreloadPinning(final boolean disablePreloadPinning) { + setDisablePreloadPinning(nativeHandle_, disablePreloadPinning); + return this; + } + + @Override + public boolean disablePreloadPinning() { + return disablePreloadPinning(nativeHandle_); + } + @Override public Options setAtomicFlush(final boolean atomicFlush) { setAtomicFlush(nativeHandle_, atomicFlush); @@ -2390,6 +2401,9 @@ private native void setCompactionOptionsFIFO(final long handle, private native void setForceConsistencyChecks(final long handle, final boolean forceConsistencyChecks); private native boolean forceConsistencyChecks(final long handle); + private native void setDisablePreloadPinning(final long handle, + final boolean disablePreloadPinning); + private native boolean disablePreloadPinning(final long handle); private native void setAtomicFlush(final long handle, final boolean atomicFlush); private native boolean atomicFlush(final long handle); diff --git a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java index 830df3f8a6b..851e6b96c26 100644 --- a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java +++ b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java @@ -625,6 +625,16 @@ public void forceConsistencyChecks() { } } + @Test + public void disablePreloadPinning() { + try (final ColumnFamilyOptions opt = new ColumnFamilyOptions()) { + final boolean booleanValue = true; + opt.setDisablePreloadPinning(booleanValue); + assertThat(opt.disablePreloadPinning()). + isEqualTo(booleanValue); + } + } + @Test public void compactionFilter() { try(final ColumnFamilyOptions options = new ColumnFamilyOptions(); diff --git a/java/src/test/java/org/rocksdb/OptionsTest.java b/java/src/test/java/org/rocksdb/OptionsTest.java index e402cb4748a..e2451b61e1d 100644 --- a/java/src/test/java/org/rocksdb/OptionsTest.java +++ b/java/src/test/java/org/rocksdb/OptionsTest.java @@ -1292,6 +1292,16 @@ public void forceConsistencyChecks() { } } + @Test + public void disablePreloadPinning() { + try (final Options options = new Options()) { + final boolean booleanValue = true; + options.setDisablePreloadPinning(booleanValue); + assertThat(options.disablePreloadPinning()). + isEqualTo(booleanValue); + } + } + @Test public void compactionFilter() { try(final Options options = new Options(); diff --git a/options/cf_options.cc b/options/cf_options.cc index 3f2b8bb7320..18089dfba89 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -485,6 +485,10 @@ static std::unordered_map {"compaction_measure_io_stats", {0, OptionType::kBoolean, OptionVerificationType::kDeprecated, OptionTypeFlags::kNone}}, + {"disable_preload_pinning", + {offset_of(&ImmutableCFOptions::disable_preload_pinning), + OptionType::kBoolean, OptionVerificationType::kNormal, + OptionTypeFlags::kNone}}, {"inplace_update_support", {offset_of(&ImmutableCFOptions::inplace_update_support), OptionType::kBoolean, OptionVerificationType::kNormal, @@ -823,6 +827,7 @@ ImmutableCFOptions::ImmutableCFOptions(const ColumnFamilyOptions& cf_options) num_levels(cf_options.num_levels), optimize_filters_for_hits(cf_options.optimize_filters_for_hits), force_consistency_checks(cf_options.force_consistency_checks), + disable_preload_pinning(cf_options.disable_preload_pinning), memtable_insert_with_hint_prefix_extractor( cf_options.memtable_insert_with_hint_prefix_extractor), cf_paths(cf_options.cf_paths), diff --git a/options/cf_options.h b/options/cf_options.h index d4e77f04f7b..226b0c4aef4 100644 --- a/options/cf_options.h +++ b/options/cf_options.h @@ -74,6 +74,8 @@ struct ImmutableCFOptions { bool force_consistency_checks; + bool disable_preload_pinning; + std::shared_ptr memtable_insert_with_hint_prefix_extractor; diff --git a/options/options.cc b/options/options.cc index 4faee64b4b1..d3f981af339 100644 --- a/options/options.cc +++ b/options/options.cc @@ -85,6 +85,7 @@ AdvancedColumnFamilyOptions::AdvancedColumnFamilyOptions(const Options& options) optimize_filters_for_hits(options.optimize_filters_for_hits), paranoid_file_checks(options.paranoid_file_checks), force_consistency_checks(options.force_consistency_checks), + disable_preload_pinning(options.disable_preload_pinning), report_bg_io_stats(options.report_bg_io_stats), ttl(options.ttl), periodic_compaction_seconds(options.periodic_compaction_seconds), @@ -378,6 +379,8 @@ void ColumnFamilyOptions::Dump(Logger* log) const { paranoid_file_checks); ROCKS_LOG_HEADER(log, " Options.force_consistency_checks: %d", force_consistency_checks); + ROCKS_LOG_HEADER(log, " Options.disable_preload_pinning: %d", + disable_preload_pinning); ROCKS_LOG_HEADER(log, " Options.report_bg_io_stats: %d", report_bg_io_stats); ROCKS_LOG_HEADER(log, " Options.ttl: %" PRIu64, diff --git a/options/options_helper.cc b/options/options_helper.cc index a554ca358a8..e73710e8e29 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -294,6 +294,7 @@ void UpdateColumnFamilyOptions(const ImmutableCFOptions& ioptions, cf_opts->num_levels = ioptions.num_levels; cf_opts->optimize_filters_for_hits = ioptions.optimize_filters_for_hits; cf_opts->force_consistency_checks = ioptions.force_consistency_checks; + cf_opts->disable_preload_pinning = ioptions.disable_preload_pinning; cf_opts->memtable_insert_with_hint_prefix_extractor = ioptions.memtable_insert_with_hint_prefix_extractor; cf_opts->cf_paths = ioptions.cf_paths; diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc index 3cfa2ecffef..2a3fdef0f26 100644 --- a/options/options_settable_test.cc +++ b/options/options_settable_test.cc @@ -496,6 +496,7 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { "check_flush_compaction_key_order=false;" "paranoid_file_checks=true;" "force_consistency_checks=true;" + "disable_preload_pinning=true;" "inplace_update_num_locks=7429;" "optimize_filters_for_hits=false;" "level_compaction_dynamic_level_bytes=false;" diff --git a/options/options_test.cc b/options/options_test.cc index aa152257cc9..856ff8bf61f 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -1499,6 +1499,10 @@ TEST_F(OptionsTest, MutableCFOptions) { ASSERT_NOK(GetColumnFamilyOptionsFromMap( config_options, cf_opts, {{"force_consistency_checks", "true"}}, &cf_opts)); + // disable preload pinning is not mutable + ASSERT_NOK(GetColumnFamilyOptionsFromMap( + config_options, cf_opts, {{"disable_preload_pinning", "true"}}, + &cf_opts)); // Attempt to change the table. It is not mutable, so this should fail and // leave the original intact diff --git a/test_util/testutil.cc b/test_util/testutil.cc index 450598cecb9..490602a240e 100644 --- a/test_util/testutil.cc +++ b/test_util/testutil.cc @@ -382,6 +382,7 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, DBOptions& db_options, cf_opt->paranoid_file_checks = rnd->Uniform(2); cf_opt->purge_redundant_kvs_while_flush = rnd->Uniform(2); cf_opt->force_consistency_checks = rnd->Uniform(2); + cf_opt->disable_preload_pinning = rnd->Uniform(2); cf_opt->compaction_options_fifo.allow_compaction = rnd->Uniform(2); cf_opt->memtable_whole_key_filtering = rnd->Uniform(2); cf_opt->enable_blob_files = rnd->Uniform(2); diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 118f398022a..6fe8d2ef958 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -64,6 +64,8 @@ const std::string LDBCommand::ARG_TIMESTAMP = "timestamp"; const std::string LDBCommand::ARG_TRY_LOAD_OPTIONS = "try_load_options"; const std::string LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS = "disable_consistency_checks"; +const std::string LDBCommand::ARG_DISABLE_PRELOAD_PINNING = + "disable_preload_pinning"; const std::string LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS = "ignore_unknown_options"; const std::string LDBCommand::ARG_FROM = "from"; @@ -378,6 +380,8 @@ LDBCommand::LDBCommand(const std::map& options, try_load_options_ = IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS); force_consistency_checks_ = !IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS); + disable_preload_pinning_ = + IsFlagPresent(flags, ARG_DISABLE_PRELOAD_PINNING); config_options_.ignore_unknown_options = IsFlagPresent(flags, ARG_IGNORE_UNKNOWN_OPTIONS); } @@ -503,6 +507,7 @@ std::vector LDBCommand::BuildCmdLineOptions( ARG_FIX_PREFIX_LEN, ARG_TRY_LOAD_OPTIONS, ARG_DISABLE_CONSISTENCY_CHECKS, + ARG_DISABLE_PRELOAD_PINNING, ARG_IGNORE_UNKNOWN_OPTIONS, ARG_CF_NAME}; ret.insert(ret.end(), options.begin(), options.end()); @@ -605,6 +610,7 @@ void LDBCommand::OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) { } cf_opts->force_consistency_checks = force_consistency_checks_; + cf_opts->disable_preload_pinning = disable_preload_pinning_; if (use_table_options) { cf_opts->table_factory.reset(NewBlockBasedTableFactory(table_options)); } diff --git a/tools/ldb_tool.cc b/tools/ldb_tool.cc index f8f7e718198..165e14c069e 100644 --- a/tools/ldb_tool.cc +++ b/tools/ldb_tool.cc @@ -49,6 +49,8 @@ void LDBCommandRunner::PrintHelp(const LDBOptions& ldb_options, " : Try to load option file from DB.\n"); ret.append(" --" + LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS + " : Set options.force_consistency_checks = false.\n"); + ret.append(" --" + LDBCommand::ARG_DISABLE_PRELOAD_PINNING + + " : Set options.disable_preload_pinning = true.\n"); ret.append(" --" + LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS + " : Ignore unknown options when loading option file.\n"); ret.append(" --" + LDBCommand::ARG_BLOOM_BITS + "=\n"); From f530d057c1366b57e53f5c7a73068da25cd0015b Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 25 Jun 2021 11:21:32 -0400 Subject: [PATCH 02/39] remove testing assert() that needs to NOT be there --- db/version_builder.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 5fd61de16ee..fc8ed63987e 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -922,7 +922,6 @@ class VersionBuilder::Rep { const SliceTransform* prefix_extractor, size_t max_file_size_for_l0_meta_pin) { assert(table_cache_ != nullptr); - assert(false == ioptions_->disable_preload_pinning); size_t table_cache_capacity = table_cache_->get_cache()->GetCapacity(); bool always_load = (table_cache_capacity == TableCache::kInfiniteCapacity); From 8d57520db40511ae1ffa0ce0337e174d8f0bdbd6 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 25 Jun 2021 11:50:05 -0400 Subject: [PATCH 03/39] build_tools/format-diff.sh: required whitespace changes --- db/version_builder.cc | 2 +- java/rocksjni/options.cc | 4 ++-- .../org/rocksdb/AdvancedColumnFamilyOptionsInterface.java | 3 +-- java/src/main/java/org/rocksdb/ColumnFamilyOptions.java | 4 ++-- java/src/main/java/org/rocksdb/Options.java | 4 ++-- java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java | 3 +-- java/src/test/java/org/rocksdb/OptionsTest.java | 3 +-- tools/ldb_cmd.cc | 3 +-- 8 files changed, 11 insertions(+), 15 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index fc8ed63987e..8ec075267f1 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -1001,7 +1001,7 @@ class VersionBuilder::Rep { if (file_meta->table_reader_handle != nullptr) { if (!ioptions_->disable_preload_pinning) { file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( - file_meta->table_reader_handle); + file_meta->table_reader_handle); } else { table_cache_->ReleaseHandle(file_meta->table_reader_handle); file_meta->table_reader_handle = nullptr; diff --git a/java/rocksjni/options.cc b/java/rocksjni/options.cc index 1fc9daa2abf..9d19822c2ca 100644 --- a/java/rocksjni/options.cc +++ b/java/rocksjni/options.cc @@ -3690,8 +3690,8 @@ jboolean Java_org_rocksdb_Options_forceConsistencyChecks( * Method: disablePreloadPinning * Signature: (J)Z */ -jboolean Java_org_rocksdb_Options_disablePreloadPinning( - JNIEnv*, jobject, jlong jhandle) { +jboolean Java_org_rocksdb_Options_disablePreloadPinning(JNIEnv*, jobject, + jlong jhandle) { auto* opts = reinterpret_cast(jhandle); return static_cast(opts->disable_preload_pinning); } diff --git a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java index 4f33843cadf..1a9c5a5ed1b 100644 --- a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java +++ b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java @@ -477,8 +477,7 @@ T setForceConsistencyChecks( * * @return the reference to the current options. */ - T setDisablePreloadPinning( - boolean disablePreloadPinning); + T setDisablePreloadPinning(boolean disablePreloadPinning); /** * RocksDB uses the first 25% of num_open_files for precaching during diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java index b18a0e151fe..459950ac40e 100644 --- a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java +++ b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java @@ -1101,8 +1101,8 @@ private native void setCompactionOptionsFIFO(final long handle, private native void setForceConsistencyChecks(final long handle, final boolean forceConsistencyChecks); private native boolean forceConsistencyChecks(final long handle); - private native void setDisablePreloadPinning(final long handle, - final boolean disablePreloadPinning); + private native void setDisablePreloadPinning( + final long handle, final boolean disablePreloadPinning); private native boolean disablePreloadPinning(final long handle); private native void setSstPartitionerFactory(long nativeHandle_, long newFactoryHandle); private static native void setCompactionThreadLimiter( diff --git a/java/src/main/java/org/rocksdb/Options.java b/java/src/main/java/org/rocksdb/Options.java index e31017650d2..16485f92fe3 100644 --- a/java/src/main/java/org/rocksdb/Options.java +++ b/java/src/main/java/org/rocksdb/Options.java @@ -2401,8 +2401,8 @@ private native void setCompactionOptionsFIFO(final long handle, private native void setForceConsistencyChecks(final long handle, final boolean forceConsistencyChecks); private native boolean forceConsistencyChecks(final long handle); - private native void setDisablePreloadPinning(final long handle, - final boolean disablePreloadPinning); + private native void setDisablePreloadPinning( + final long handle, final boolean disablePreloadPinning); private native boolean disablePreloadPinning(final long handle); private native void setAtomicFlush(final long handle, final boolean atomicFlush); diff --git a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java index 851e6b96c26..3653e52561b 100644 --- a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java +++ b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java @@ -630,8 +630,7 @@ public void disablePreloadPinning() { try (final ColumnFamilyOptions opt = new ColumnFamilyOptions()) { final boolean booleanValue = true; opt.setDisablePreloadPinning(booleanValue); - assertThat(opt.disablePreloadPinning()). - isEqualTo(booleanValue); + assertThat(opt.disablePreloadPinning()).isEqualTo(booleanValue); } } diff --git a/java/src/test/java/org/rocksdb/OptionsTest.java b/java/src/test/java/org/rocksdb/OptionsTest.java index e2451b61e1d..70c6daf3fdb 100644 --- a/java/src/test/java/org/rocksdb/OptionsTest.java +++ b/java/src/test/java/org/rocksdb/OptionsTest.java @@ -1297,8 +1297,7 @@ public void disablePreloadPinning() { try (final Options options = new Options()) { final boolean booleanValue = true; options.setDisablePreloadPinning(booleanValue); - assertThat(options.disablePreloadPinning()). - isEqualTo(booleanValue); + assertThat(options.disablePreloadPinning()).isEqualTo(booleanValue); } } diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 6fe8d2ef958..68df372a03d 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -380,8 +380,7 @@ LDBCommand::LDBCommand(const std::map& options, try_load_options_ = IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS); force_consistency_checks_ = !IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS); - disable_preload_pinning_ = - IsFlagPresent(flags, ARG_DISABLE_PRELOAD_PINNING); + disable_preload_pinning_ = IsFlagPresent(flags, ARG_DISABLE_PRELOAD_PINNING); config_options_.ignore_unknown_options = IsFlagPresent(flags, ARG_IGNORE_UNKNOWN_OPTIONS); } From 3c09932eb9ba28c1b2bedfa2d6a8992398e85275 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 25 Jun 2021 12:01:36 -0400 Subject: [PATCH 04/39] correct capitalization of disablePreloadPinning() function name --- .../java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java index 1a9c5a5ed1b..1d13d14387b 100644 --- a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java +++ b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java @@ -486,5 +486,5 @@ T setForceConsistencyChecks( * * @return true when pinning preloaded files into cache is disable */ - boolean DisablePreloadPinning(); + boolean disablePreloadPinning(); } From 460b303afcf8dc4c0c573bf06eb714f5d73a4185 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 4 Jan 2022 14:19:13 -0500 Subject: [PATCH 05/39] declare enum FilePreload and option file_preload --- include/rocksdb/advanced_options.h | 33 ++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index dce179a1c2a..3285db57585 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -56,6 +56,25 @@ enum CompactionPri : char { kMinOverlappingRatio = 0x3, }; +// RocksDB uses the first 25% of num_open_files for precaching during +// start-up and after compactions. The files precached in this fashion +// provide faster access. However, these files are also never released. +// Scenarios that have large bloom filters not cached or scenarios where +// user is manually lowering the num_open_files at runtime might want +// to disable this behavior. +enum FilePreload : char { + // RocksDB uses the first 25% of num_open_files for precaching during + // start-up and after compactions. The files precached in this fashion + // provide faster access. However, these files are also never released. + kFilePreloadWithPinning = 0x0, + // RocksDB uses the first 25% of num_open_files for precaching during + // start-up and after compactions. No pinning within cache, so access + // has one additional layer of indirection. But cache space can free. + kFilePreloadWithoutPinning = 0x1, + // RocksDB does not open existing table files during start-up. + kFilePreloadDisabled = 0x2, +}; + struct CompactionOptionsFIFO { // once the total sum of table files reaches this, we will delete the oldest // table file @@ -703,14 +722,12 @@ struct AdvancedColumnFamilyOptions { // Default: true bool force_consistency_checks = true; - // RocksDB uses the first 25% of num_open_files for precaching during - // start-up and after compactions. The files precached in this fashion - // provide faster access. However, these files are also never released. - // Scenarios that have large bloom filters not cached or scenarios where - // user is manually lowering the num_open_files at runtime might want - // to disable this behavior. - // Default: false - bool disable_preload_pinning = false; + // RocksDB can preload and optionally pin table files within the table + // cache at start-up and after compactions. The files precached in this + // fashion provide faster access. However, these files are also never + // released from the table cache. + // Default: kFilePreloadWithPinning + FilePreload file_preload = kFilePreloadWithPinning; // Measure IO stats in compactions and flushes, if true. // From 4096a40067a774ef35e69839535bf62eb6f9e295 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 4 Jan 2022 14:21:36 -0500 Subject: [PATCH 06/39] add kFilePreload option type --- include/rocksdb/utilities/options_type.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/rocksdb/utilities/options_type.h b/include/rocksdb/utilities/options_type.h index d7d38898507..ddad58dc28f 100644 --- a/include/rocksdb/utilities/options_type.h +++ b/include/rocksdb/utilities/options_type.h @@ -38,6 +38,7 @@ enum class OptionType { kCompactionFilter, kCompactionFilterFactory, kCompactionStopStyle, + kFilePreload, kMergeOperator, kMemTableRepFactory, kFilterPolicy, From 2302ee84599e9add36b8f4ece48ba8473a5db031 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 4 Jan 2022 19:46:36 -0500 Subject: [PATCH 07/39] if options file creation fails, get rid of its temporary file. --- db/db_impl/db_impl.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 87d919ac2ad..7965f290fd6 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -4102,6 +4102,10 @@ Status DBImpl::WriteOptionsFile(bool need_mutex_lock, if (s.ok()) { s = RenameTempFileToOptionsFile(file_name); + } else { + // do not accumulate failed files + // (could be hundreds with bad options code) + (void)env_->DeleteFile(file_name).ok(); } // restore lock if (!need_mutex_lock) { From 47b05cc0e18769160de6fd521d98e7612e74e088 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 4 Jan 2022 20:34:40 -0500 Subject: [PATCH 08/39] utilize new kFilePreload options to determine if files should open now or later --- db/version_builder.cc | 12 +++++++++++- db/version_edit_handler.cc | 6 ++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 8ec075267f1..573ef4b488c 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -926,6 +926,16 @@ class VersionBuilder::Rep { size_t table_cache_capacity = table_cache_->get_cache()->GetCapacity(); bool always_load = (table_cache_capacity == TableCache::kInfiniteCapacity); size_t max_load = port::kMaxSizet; +#ifndef NDEBUG + bool debug_override = true; // to enable CompactedDB related tests and some property tests +#else + bool debug_override = false; +#endif + (void) debug_override; + if (FilePreload::kFilePreloadDisabled == ioptions_->file_preload) { + max_load = 0; + always_load = true; + } if (!always_load) { // If it is initial loading and not set to always loading all the @@ -999,7 +1009,7 @@ class VersionBuilder::Rep { // table cache which over time are no longer useful. The // disable_preload_pinning option keeps #1 and disables #2. if (file_meta->table_reader_handle != nullptr) { - if (!ioptions_->disable_preload_pinning) { + if (ioptions_->file_preload == kFilePreloadWithPinning) { file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( file_meta->table_reader_handle); } else { diff --git a/db/version_edit_handler.cc b/db/version_edit_handler.cc index 7a2996a59e2..75b086dda55 100644 --- a/db/version_edit_handler.cc +++ b/db/version_edit_handler.cc @@ -495,7 +495,8 @@ Status VersionEditHandler::MaybeCreateVersion(const VersionEdit& /*edit*/, // Install new version v->PrepareApply( *cfd->GetLatestMutableCFOptions(), - !(version_set_->db_options_->skip_stats_update_on_db_open)); + !(version_set_->db_options_->skip_stats_update_on_db_open || + kFilePreloadDisabled == cfd->ioptions()->file_preload)); version_set_->AppendVersion(cfd, v); } else { delete v; @@ -755,7 +756,8 @@ Status VersionEditHandlerPointInTime::MaybeCreateVersion( if (s.ok()) { version->PrepareApply( *cfd->GetLatestMutableCFOptions(), - !version_set_->db_options_->skip_stats_update_on_db_open); + !(version_set_->db_options_->skip_stats_update_on_db_open || + kFilePreloadDisabled == cfd->ioptions()->file_preload)); auto v_iter = versions_.find(cfd->GetID()); if (v_iter != versions_.end()) { delete v_iter->second; From 7b3378f269f561fbd6b71bdc2b45602e8b102e64 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 4 Jan 2022 20:37:37 -0500 Subject: [PATCH 09/39] add file_preload option the the serialization/deserialization logic and tests --- options/cf_options.cc | 8 ++++---- options/cf_options.h | 2 +- options/options.cc | 6 +++--- options/options_helper.cc | 23 ++++++++++++++++++++++- options/options_helper.h | 3 +++ options/options_settable_test.cc | 2 +- options/options_test.cc | 4 ++-- 7 files changed, 36 insertions(+), 12 deletions(-) diff --git a/options/cf_options.cc b/options/cf_options.cc index 18089dfba89..6567cf17511 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -485,9 +485,9 @@ static std::unordered_map {"compaction_measure_io_stats", {0, OptionType::kBoolean, OptionVerificationType::kDeprecated, OptionTypeFlags::kNone}}, - {"disable_preload_pinning", - {offset_of(&ImmutableCFOptions::disable_preload_pinning), - OptionType::kBoolean, OptionVerificationType::kNormal, + {"file_preload", + {offset_of(&ImmutableCFOptions::file_preload), + OptionType::kFilePreload, OptionVerificationType::kNormal, OptionTypeFlags::kNone}}, {"inplace_update_support", {offset_of(&ImmutableCFOptions::inplace_update_support), @@ -827,7 +827,7 @@ ImmutableCFOptions::ImmutableCFOptions(const ColumnFamilyOptions& cf_options) num_levels(cf_options.num_levels), optimize_filters_for_hits(cf_options.optimize_filters_for_hits), force_consistency_checks(cf_options.force_consistency_checks), - disable_preload_pinning(cf_options.disable_preload_pinning), + file_preload(cf_options.file_preload), memtable_insert_with_hint_prefix_extractor( cf_options.memtable_insert_with_hint_prefix_extractor), cf_paths(cf_options.cf_paths), diff --git a/options/cf_options.h b/options/cf_options.h index 226b0c4aef4..94267c043c4 100644 --- a/options/cf_options.h +++ b/options/cf_options.h @@ -74,7 +74,7 @@ struct ImmutableCFOptions { bool force_consistency_checks; - bool disable_preload_pinning; + FilePreload file_preload; std::shared_ptr memtable_insert_with_hint_prefix_extractor; diff --git a/options/options.cc b/options/options.cc index d3f981af339..4fc64cf157e 100644 --- a/options/options.cc +++ b/options/options.cc @@ -85,7 +85,7 @@ AdvancedColumnFamilyOptions::AdvancedColumnFamilyOptions(const Options& options) optimize_filters_for_hits(options.optimize_filters_for_hits), paranoid_file_checks(options.paranoid_file_checks), force_consistency_checks(options.force_consistency_checks), - disable_preload_pinning(options.disable_preload_pinning), + file_preload(options.file_preload), report_bg_io_stats(options.report_bg_io_stats), ttl(options.ttl), periodic_compaction_seconds(options.periodic_compaction_seconds), @@ -379,8 +379,8 @@ void ColumnFamilyOptions::Dump(Logger* log) const { paranoid_file_checks); ROCKS_LOG_HEADER(log, " Options.force_consistency_checks: %d", force_consistency_checks); - ROCKS_LOG_HEADER(log, " Options.disable_preload_pinning: %d", - disable_preload_pinning); + ROCKS_LOG_HEADER(log, " Options.file_preload: %d", + file_preload); ROCKS_LOG_HEADER(log, " Options.report_bg_io_stats: %d", report_bg_io_stats); ROCKS_LOG_HEADER(log, " Options.ttl: %" PRIu64, diff --git a/options/options_helper.cc b/options/options_helper.cc index e73710e8e29..958c8387607 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -294,7 +294,7 @@ void UpdateColumnFamilyOptions(const ImmutableCFOptions& ioptions, cf_opts->num_levels = ioptions.num_levels; cf_opts->optimize_filters_for_hits = ioptions.optimize_filters_for_hits; cf_opts->force_consistency_checks = ioptions.force_consistency_checks; - cf_opts->disable_preload_pinning = ioptions.disable_preload_pinning; + cf_opts->file_preload = ioptions.file_preload; cf_opts->memtable_insert_with_hint_prefix_extractor = ioptions.memtable_insert_with_hint_prefix_extractor; cf_opts->cf_paths = ioptions.cf_paths; @@ -318,6 +318,11 @@ std::map OptionsHelper::compaction_pri_to_string = { {kOldestSmallestSeqFirst, "kOldestSmallestSeqFirst"}, {kMinOverlappingRatio, "kMinOverlappingRatio"}}; +std::map OptionsHelper::file_preload_to_string = { + {kFilePreloadWithPinning, "kFilePreloadWithPinning"}, + {kFilePreloadWithoutPinning, "kFilePreloadWithoutPinning"}, + {kFilePreloadDisabled, "kFilePreloadDisabled"}}; + std::map OptionsHelper::compaction_stop_style_to_string = { {kCompactionStopStyleSimilarSize, "kCompactionStopStyleSimilarSize"}, @@ -467,6 +472,10 @@ static bool ParseOptionHelper(void* opt_address, const OptionType& opt_type, return ParseEnum( compression_type_string_map, value, static_cast(opt_address)); + case OptionType::kFilePreload: + return ParseEnum( + file_preload_string_map, value, + reinterpret_cast(opt_address)); case OptionType::kSliceTransform: return ParseSliceTransform( value, @@ -555,6 +564,10 @@ bool SerializeSingleOptionHelper(const void* opt_address, return SerializeEnum( compression_type_string_map, *(static_cast(opt_address)), value); + case OptionType::kFilePreload: + return SerializeEnum( + file_preload_string_map, + *(reinterpret_cast(opt_address)), value); case OptionType::kSliceTransform: { const auto* slice_transform_ptr = static_cast*>( @@ -907,6 +920,12 @@ std::unordered_map {"kCompactionStopStyleSimilarSize", kCompactionStopStyleSimilarSize}, {"kCompactionStopStyleTotalSize", kCompactionStopStyleTotalSize}}; +std::unordered_map + OptionsHelper::file_preload_string_map = { + {"kFilePreloadWithPinning", FilePreload::kFilePreloadWithPinning}, + {"kFilePreloadWithoutPinning", FilePreload::kFilePreloadWithoutPinning}, + {"kFilePreloadDisabled", FilePreload::kFilePreloadDisabled}}; + Status OptionTypeInfo::NextToken(const std::string& opts, char delimiter, size_t pos, size_t* end, std::string* token) { while (pos < opts.size() && isspace(opts[pos])) { @@ -1245,6 +1264,8 @@ static bool AreOptionsEqual(OptionType type, const void* this_offset, return IsOptionEqual(this_offset, that_offset); case OptionType::kCompressionType: return IsOptionEqual(this_offset, that_offset); + case OptionType::kFilePreload: + return IsOptionEqual(this_offset, that_offset); case OptionType::kChecksumType: return IsOptionEqual(this_offset, that_offset); case OptionType::kEncodingType: diff --git a/options/options_helper.h b/options/options_helper.h index a16c265ede9..6692ffaad78 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -83,6 +83,8 @@ struct OptionsHelper { static std::unordered_map compaction_pri_string_map; #endif // !ROCKSDB_LITE + static std::map file_preload_to_string; + static std::unordered_map file_preload_string_map; }; // Some aliasing @@ -103,5 +105,6 @@ static auto& compaction_style_string_map = static auto& compaction_pri_string_map = OptionsHelper::compaction_pri_string_map; #endif // !ROCKSDB_LITE +static auto& file_preload_string_map = OptionsHelper::file_preload_string_map; } // namespace ROCKSDB_NAMESPACE diff --git a/options/options_settable_test.cc b/options/options_settable_test.cc index 2a3fdef0f26..0d9c6833f1d 100644 --- a/options/options_settable_test.cc +++ b/options/options_settable_test.cc @@ -496,7 +496,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { "check_flush_compaction_key_order=false;" "paranoid_file_checks=true;" "force_consistency_checks=true;" - "disable_preload_pinning=true;" "inplace_update_num_locks=7429;" "optimize_filters_for_hits=false;" "level_compaction_dynamic_level_bytes=false;" @@ -515,6 +514,7 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) { "blob_compression_type=kBZip2Compression;" "enable_blob_garbage_collection=true;" "blob_garbage_collection_age_cutoff=0.5;" + "file_preload=kFilePreloadWithPinning;" "compaction_options_fifo={max_table_files_size=3;allow_" "compaction=false;};", new_options)); diff --git a/options/options_test.cc b/options/options_test.cc index 856ff8bf61f..5ea9bd7c5ca 100644 --- a/options/options_test.cc +++ b/options/options_test.cc @@ -1499,9 +1499,9 @@ TEST_F(OptionsTest, MutableCFOptions) { ASSERT_NOK(GetColumnFamilyOptionsFromMap( config_options, cf_opts, {{"force_consistency_checks", "true"}}, &cf_opts)); - // disable preload pinning is not mutable + // file_preload is not mutable ASSERT_NOK(GetColumnFamilyOptionsFromMap( - config_options, cf_opts, {{"disable_preload_pinning", "true"}}, + config_options, cf_opts, {{"file_preload", "kFilePreloadWithPinning"}}, &cf_opts)); // Attempt to change the table. It is not mutable, so this should fail and From 1d9c4c24d7b2fb6ee9651ff5df11c780ba2daa6a Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 4 Jan 2022 20:39:45 -0500 Subject: [PATCH 10/39] add file_preload options to ldb cmd and tool. --- include/rocksdb/utilities/ldb_cmd.h | 6 +++--- tools/ldb_cmd.cc | 20 +++++++++++++++----- tools/ldb_tool.cc | 4 ++-- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/include/rocksdb/utilities/ldb_cmd.h b/include/rocksdb/utilities/ldb_cmd.h index 169f8c50c8a..70a1acd274e 100644 --- a/include/rocksdb/utilities/ldb_cmd.h +++ b/include/rocksdb/utilities/ldb_cmd.h @@ -61,7 +61,7 @@ class LDBCommand { static const std::string ARG_CREATE_IF_MISSING; static const std::string ARG_NO_VALUE; static const std::string ARG_DISABLE_CONSISTENCY_CHECKS; - static const std::string ARG_DISABLE_PRELOAD_PINNING; + static const std::string ARG_FILE_PRELOAD; struct ParsedParams { std::string cmd; @@ -174,8 +174,8 @@ class LDBCommand { // The value passed to options.force_consistency_checks. bool force_consistency_checks_; - // The value passed to options.disable_preload_pinning. - bool disable_preload_pinning_; + // The value passed to options.file_preload. + FilePreload file_preload_; bool create_if_missing_; diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 68df372a03d..121e16f132b 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -64,8 +64,8 @@ const std::string LDBCommand::ARG_TIMESTAMP = "timestamp"; const std::string LDBCommand::ARG_TRY_LOAD_OPTIONS = "try_load_options"; const std::string LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS = "disable_consistency_checks"; -const std::string LDBCommand::ARG_DISABLE_PRELOAD_PINNING = - "disable_preload_pinning"; +const std::string LDBCommand::ARG_FILE_PRELOAD = + "file_preload"; const std::string LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS = "ignore_unknown_options"; const std::string LDBCommand::ARG_FROM = "from"; @@ -380,7 +380,6 @@ LDBCommand::LDBCommand(const std::map& options, try_load_options_ = IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS); force_consistency_checks_ = !IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS); - disable_preload_pinning_ = IsFlagPresent(flags, ARG_DISABLE_PRELOAD_PINNING); config_options_.ignore_unknown_options = IsFlagPresent(flags, ARG_IGNORE_UNKNOWN_OPTIONS); } @@ -502,11 +501,12 @@ std::vector LDBCommand::BuildCmdLineOptions( ARG_COMPRESSION_TYPE, ARG_COMPRESSION_MAX_DICT_BYTES, ARG_WRITE_BUFFER_SIZE, + ARG_FILE_PRELOAD, ARG_FILE_SIZE, ARG_FIX_PREFIX_LEN, ARG_TRY_LOAD_OPTIONS, ARG_DISABLE_CONSISTENCY_CHECKS, - ARG_DISABLE_PRELOAD_PINNING, + ARG_FILE_PRELOAD, ARG_IGNORE_UNKNOWN_OPTIONS, ARG_CF_NAME}; ret.insert(ret.end(), options.begin(), options.end()); @@ -609,7 +609,6 @@ void LDBCommand::OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) { } cf_opts->force_consistency_checks = force_consistency_checks_; - cf_opts->disable_preload_pinning = disable_preload_pinning_; if (use_table_options) { cf_opts->table_factory.reset(NewBlockBasedTableFactory(table_options)); } @@ -688,6 +687,17 @@ void LDBCommand::OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) { LDBCommandExecuteResult::Failed(ARG_FIX_PREFIX_LEN + " must be > 0."); } } + + int file_preload; + if (ParseIntOption(option_map_, ARG_FILE_PRELOAD, file_preload, + exec_state_)) { + if (0 <= file_preload && file_preload < 3) { + cf_opts->file_preload = (FilePreload)file_preload; + } else { + exec_state_ = + LDBCommandExecuteResult::Failed(ARG_FILE_PRELOAD + " must be 0, 1, or 2."); + } + } } // First, initializes the options state using the OPTIONS file when enabled. diff --git a/tools/ldb_tool.cc b/tools/ldb_tool.cc index 165e14c069e..3f2534493c4 100644 --- a/tools/ldb_tool.cc +++ b/tools/ldb_tool.cc @@ -49,8 +49,8 @@ void LDBCommandRunner::PrintHelp(const LDBOptions& ldb_options, " : Try to load option file from DB.\n"); ret.append(" --" + LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS + " : Set options.force_consistency_checks = false.\n"); - ret.append(" --" + LDBCommand::ARG_DISABLE_PRELOAD_PINNING + - " : Set options.disable_preload_pinning = true.\n"); + ret.append(" --" + LDBCommand::ARG_FILE_PRELOAD + + " : Set options.file_preload = \n"); From fd9a618d0bdf728d011fb7d87ca816c766b57d79 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 4 Jan 2022 20:40:10 -0500 Subject: [PATCH 11/39] add file_preload to test randomization --- test_util/testutil.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_util/testutil.cc b/test_util/testutil.cc index 490602a240e..2e9464a77a2 100644 --- a/test_util/testutil.cc +++ b/test_util/testutil.cc @@ -382,7 +382,7 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, DBOptions& db_options, cf_opt->paranoid_file_checks = rnd->Uniform(2); cf_opt->purge_redundant_kvs_while_flush = rnd->Uniform(2); cf_opt->force_consistency_checks = rnd->Uniform(2); - cf_opt->disable_preload_pinning = rnd->Uniform(2); + cf_opt->file_preload = (FilePreload)(rnd->Uniform(3)); cf_opt->compaction_options_fifo.allow_compaction = rnd->Uniform(2); cf_opt->memtable_whole_key_filtering = rnd->Uniform(2); cf_opt->enable_blob_files = rnd->Uniform(2); From 85cf6930380e26fd10d001d704ab92f93be437c1 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Wed, 5 Jan 2022 10:46:26 -0500 Subject: [PATCH 12/39] remove preload options from java interface per Slack discussion --- java/rocksjni/options.cc | 47 ------------------- .../AdvancedColumnFamilyOptionsInterface.java | 25 ---------- .../java/org/rocksdb/ColumnFamilyOptions.java | 14 ------ java/src/main/java/org/rocksdb/Options.java | 14 ------ .../org/rocksdb/ColumnFamilyOptionsTest.java | 9 ---- .../test/java/org/rocksdb/OptionsTest.java | 9 ---- 6 files changed, 118 deletions(-) diff --git a/java/rocksjni/options.cc b/java/rocksjni/options.cc index 9d19822c2ca..fbf3241792e 100644 --- a/java/rocksjni/options.cc +++ b/java/rocksjni/options.cc @@ -3663,17 +3663,6 @@ void Java_org_rocksdb_Options_setForceConsistencyChecks( opts->force_consistency_checks = static_cast(jforce_consistency_checks); } -/* - * Class: org_rocksdb_Options - * Method: setDisablePreloadPinning - * Signature: (JZ)V - */ -void Java_org_rocksdb_Options_setDisablePreloadPinning( - JNIEnv*, jobject, jlong jhandle, jboolean jdisable_preload_pinning) { - auto* opts = reinterpret_cast(jhandle); - opts->disable_preload_pinning = static_cast(jdisable_preload_pinning); -} - /* * Class: org_rocksdb_Options * Method: forceConsistencyChecks @@ -3685,17 +3674,6 @@ jboolean Java_org_rocksdb_Options_forceConsistencyChecks( return static_cast(opts->force_consistency_checks); } -/* - * Class: org_rocksdb_Options - * Method: disablePreloadPinning - * Signature: (J)Z - */ -jboolean Java_org_rocksdb_Options_disablePreloadPinning(JNIEnv*, jobject, - jlong jhandle) { - auto* opts = reinterpret_cast(jhandle); - return static_cast(opts->disable_preload_pinning); -} - ////////////////////////////////////////////////////////////////////////////// // ROCKSDB_NAMESPACE::ColumnFamilyOptions @@ -5225,31 +5203,6 @@ jboolean Java_org_rocksdb_ColumnFamilyOptions_forceConsistencyChecks( return static_cast(cf_opts->force_consistency_checks); } -/* - * Class: org_rocksdb_ColumnFamilyOptions - * Method: setDisablePreloadPinning - * Signature: (JZ)V - */ -void Java_org_rocksdb_ColumnFamilyOptions_setDisablePreloadPinning( - JNIEnv*, jobject, jlong jhandle, jboolean jdisable_preload_pinning) { - auto* cf_opts = - reinterpret_cast(jhandle); - cf_opts->disable_preload_pinning = - static_cast(jdisable_preload_pinning); -} - -/* - * Class: org_rocksdb_ColumnFamilyOptions - * Method: disablePreloadPinning - * Signature: (J)Z - */ -jboolean Java_org_rocksdb_ColumnFamilyOptions_disablePreloadPinning( - JNIEnv*, jobject, jlong jhandle) { - auto* cf_opts = - reinterpret_cast(jhandle); - return static_cast(cf_opts->disable_preload_pinning); -} - ///////////////////////////////////////////////////////////////////// // ROCKSDB_NAMESPACE::DBOptions diff --git a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java index 1d13d14387b..76d9bde4646 100644 --- a/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java +++ b/java/src/main/java/org/rocksdb/AdvancedColumnFamilyOptionsInterface.java @@ -462,29 +462,4 @@ T setForceConsistencyChecks( * @return true if consistency checks are enforced */ boolean forceConsistencyChecks(); - - /** - * RocksDB uses the first 25% of num_open_files for precaching during - * start-up and after compactions. The files precached in this fashion - * provide faster access. However, these files are also never released. - * Scenarios that have large bloom filters not cached or scenarios where - * user is manually lowering the num_open_files at runtime might want - * to disable this behavior. - * - * Default: false - * - * @param disablePreloadPinning true to stop pinning preload files in cache - * - * @return the reference to the current options. - */ - T setDisablePreloadPinning(boolean disablePreloadPinning); - - /** - * RocksDB uses the first 25% of num_open_files for precaching during - * start-up and after compactions. The files precached in this fashion - * might or might not be pinned in the table cache. - * - * @return true when pinning preloaded files into cache is disable - */ - boolean disablePreloadPinning(); } diff --git a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java index 459950ac40e..72149bf2669 100644 --- a/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java +++ b/java/src/main/java/org/rocksdb/ColumnFamilyOptions.java @@ -895,17 +895,6 @@ public boolean forceConsistencyChecks() { return forceConsistencyChecks(nativeHandle_); } - @Override - public ColumnFamilyOptions setDisablePreloadPinning(final boolean disablePreloadPinning) { - setDisablePreloadPinning(nativeHandle_, disablePreloadPinning); - return this; - } - - @Override - public boolean disablePreloadPinning() { - return disablePreloadPinning(nativeHandle_); - } - @Override public ColumnFamilyOptions setSstPartitionerFactory(SstPartitionerFactory sstPartitionerFactory) { setSstPartitionerFactory(nativeHandle_, sstPartitionerFactory.nativeHandle_); @@ -1101,9 +1090,6 @@ private native void setCompactionOptionsFIFO(final long handle, private native void setForceConsistencyChecks(final long handle, final boolean forceConsistencyChecks); private native boolean forceConsistencyChecks(final long handle); - private native void setDisablePreloadPinning( - final long handle, final boolean disablePreloadPinning); - private native boolean disablePreloadPinning(final long handle); private native void setSstPartitionerFactory(long nativeHandle_, long newFactoryHandle); private static native void setCompactionThreadLimiter( final long nativeHandle_, final long compactionThreadLimiterHandle); diff --git a/java/src/main/java/org/rocksdb/Options.java b/java/src/main/java/org/rocksdb/Options.java index 16485f92fe3..57f3aeffbd3 100644 --- a/java/src/main/java/org/rocksdb/Options.java +++ b/java/src/main/java/org/rocksdb/Options.java @@ -1886,17 +1886,6 @@ public boolean forceConsistencyChecks() { return forceConsistencyChecks(nativeHandle_); } - @Override - public Options setDisablePreloadPinning(final boolean disablePreloadPinning) { - setDisablePreloadPinning(nativeHandle_, disablePreloadPinning); - return this; - } - - @Override - public boolean disablePreloadPinning() { - return disablePreloadPinning(nativeHandle_); - } - @Override public Options setAtomicFlush(final boolean atomicFlush) { setAtomicFlush(nativeHandle_, atomicFlush); @@ -2401,9 +2390,6 @@ private native void setCompactionOptionsFIFO(final long handle, private native void setForceConsistencyChecks(final long handle, final boolean forceConsistencyChecks); private native boolean forceConsistencyChecks(final long handle); - private native void setDisablePreloadPinning( - final long handle, final boolean disablePreloadPinning); - private native boolean disablePreloadPinning(final long handle); private native void setAtomicFlush(final long handle, final boolean atomicFlush); private native boolean atomicFlush(final long handle); diff --git a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java index 3653e52561b..830df3f8a6b 100644 --- a/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java +++ b/java/src/test/java/org/rocksdb/ColumnFamilyOptionsTest.java @@ -625,15 +625,6 @@ public void forceConsistencyChecks() { } } - @Test - public void disablePreloadPinning() { - try (final ColumnFamilyOptions opt = new ColumnFamilyOptions()) { - final boolean booleanValue = true; - opt.setDisablePreloadPinning(booleanValue); - assertThat(opt.disablePreloadPinning()).isEqualTo(booleanValue); - } - } - @Test public void compactionFilter() { try(final ColumnFamilyOptions options = new ColumnFamilyOptions(); diff --git a/java/src/test/java/org/rocksdb/OptionsTest.java b/java/src/test/java/org/rocksdb/OptionsTest.java index 70c6daf3fdb..e402cb4748a 100644 --- a/java/src/test/java/org/rocksdb/OptionsTest.java +++ b/java/src/test/java/org/rocksdb/OptionsTest.java @@ -1292,15 +1292,6 @@ public void forceConsistencyChecks() { } } - @Test - public void disablePreloadPinning() { - try (final Options options = new Options()) { - final boolean booleanValue = true; - options.setDisablePreloadPinning(booleanValue); - assertThat(options.disablePreloadPinning()).isEqualTo(booleanValue); - } - } - @Test public void compactionFilter() { try(final Options options = new Options(); From 28f30950f50f33aa196bece8d8f17773e1c5e134 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Wed, 5 Jan 2022 10:47:10 -0500 Subject: [PATCH 13/39] update comments and appropriate use of debug_override --- db/version_builder.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 573ef4b488c..f7f783117fb 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -931,7 +931,6 @@ class VersionBuilder::Rep { #else bool debug_override = false; #endif - (void) debug_override; if (FilePreload::kFilePreloadDisabled == ioptions_->file_preload) { max_load = 0; always_load = true; @@ -1007,12 +1006,12 @@ class VersionBuilder::Rep { // 2. create higher performance via a cache lookup avoidance // The issue is that number 2 creates permanent objects in the // table cache which over time are no longer useful. The - // disable_preload_pinning option keeps #1 and disables #2. + // kFilePreloadWithoutPinning option keeps #1 and disables #2. if (file_meta->table_reader_handle != nullptr) { - if (ioptions_->file_preload == kFilePreloadWithPinning) { + if (ioptions_->file_preload == kFilePreloadWithPinning || debug_override) { file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( file_meta->table_reader_handle); - } else { + } else { // kFilePreloadWithoutPinning table_cache_->ReleaseHandle(file_meta->table_reader_handle); file_meta->table_reader_handle = nullptr; } From 2539e4e8ac0df10addcc0250093d0bc7b9aa3e87 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Wed, 5 Jan 2022 10:54:23 -0500 Subject: [PATCH 14/39] white space changes to make Check buck targets happy --- table/cuckoo/cuckoo_table_reader.h | 6 +- tools/ldb_cmd.cc | 128 ++++++++++++++--------------- 2 files changed, 63 insertions(+), 71 deletions(-) diff --git a/table/cuckoo/cuckoo_table_reader.h b/table/cuckoo/cuckoo_table_reader.h index 43afd4fd7e8..02b4723cc2b 100644 --- a/table/cuckoo/cuckoo_table_reader.h +++ b/table/cuckoo/cuckoo_table_reader.h @@ -9,8 +9,8 @@ #pragma once #ifndef ROCKSDB_LITE -#include #include +#include #include #include @@ -26,7 +26,7 @@ class Arena; class TableReader; struct ImmutableOptions; -class CuckooTableReader: public TableReader { +class CuckooTableReader : public TableReader { public: CuckooTableReader(const ImmutableOptions& ioptions, std::unique_ptr&& file, @@ -94,7 +94,7 @@ class CuckooTableReader: public TableReader { uint64_t table_size_; const Comparator* ucomp_; uint64_t (*get_slice_hash_)(const Slice& s, uint32_t index, - uint64_t max_num_buckets); + uint64_t max_num_buckets); }; } // namespace ROCKSDB_NAMESPACE diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 121e16f132b..1ecbeefcda3 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -64,8 +64,7 @@ const std::string LDBCommand::ARG_TIMESTAMP = "timestamp"; const std::string LDBCommand::ARG_TRY_LOAD_OPTIONS = "try_load_options"; const std::string LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS = "disable_consistency_checks"; -const std::string LDBCommand::ARG_FILE_PRELOAD = - "file_preload"; +const std::string LDBCommand::ARG_FILE_PRELOAD = "file_preload"; const std::string LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS = "ignore_unknown_options"; const std::string LDBCommand::ARG_FROM = "from"; @@ -94,7 +93,7 @@ void DumpWalFile(Options options, std::string wal_file, bool print_header, void DumpSstFile(Options options, std::string filename, bool output_hex, bool show_properties); -}; +}; // namespace LDBCommand* LDBCommand::InitFromCmdLineArgs( int argc, char const* const* argv, const Options& options, @@ -137,7 +136,7 @@ LDBCommand* LDBCommand::InitFromCmdLineArgs( const std::string OPTION_PREFIX = "--"; for (const auto& arg : args) { - if (arg[0] == '-' && arg[1] == '-'){ + if (arg[0] == '-' && arg[1] == '-') { std::vector splits = StringSplit(arg, '='); // --option_name=option_value if (splits.size() == 2) { @@ -259,8 +258,7 @@ LDBCommand* LDBCommand::SelectCommand(const ParsedParams& parsed_params) { parsed_params.flags); } else if (parsed_params.cmd == CheckPointCommand::Name()) { return new CheckPointCommand(parsed_params.cmd_params, - parsed_params.option_map, - parsed_params.flags); + parsed_params.option_map, parsed_params.flags); } else if (parsed_params.cmd == RepairCommand::Name()) { return new RepairCommand(parsed_params.cmd_params, parsed_params.option_map, parsed_params.flags); @@ -657,7 +655,7 @@ void LDBCommand::OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) { int write_buffer_size; if (ParseIntOption(option_map_, ARG_WRITE_BUFFER_SIZE, write_buffer_size, - exec_state_)) { + exec_state_)) { if (write_buffer_size > 0) { cf_opts->write_buffer_size = write_buffer_size; } else { @@ -694,8 +692,8 @@ void LDBCommand::OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) { if (0 <= file_preload && file_preload < 3) { cf_opts->file_preload = (FilePreload)file_preload; } else { - exec_state_ = - LDBCommandExecuteResult::Failed(ARG_FILE_PRELOAD + " must be 0, 1, or 2."); + exec_state_ = LDBCommandExecuteResult::Failed(ARG_FILE_PRELOAD + + " must be 0, 1, or 2."); } } } @@ -1047,7 +1045,7 @@ void DBLoaderCommand::DoCommand() { } else if (0 == line.find("Created bg thread 0x")) { // ignore this line } else { - bad_lines ++; + bad_lines++; } } @@ -1137,7 +1135,6 @@ ManifestDumpCommand::ManifestDumpCommand( } void ManifestDumpCommand::DoCommand() { - std::string manifestfile; if (!path_.empty()) { @@ -1493,7 +1490,7 @@ void IncBucketCounts(std::vector& bucket_counts, int ttl_start, (void)num_buckets; #endif assert(time_range > 0 && timekv >= ttl_start && bucket_size > 0 && - timekv < (ttl_start + time_range) && num_buckets > 1); + timekv < (ttl_start + time_range) && num_buckets > 1); int bucket = (timekv - ttl_start) / bucket_size; bucket_counts[bucket]++; } @@ -1502,7 +1499,7 @@ void PrintBucketCounts(const std::vector& bucket_counts, int ttl_start, int ttl_end, int bucket_size, int num_buckets) { int time_point = ttl_start; - for(int i = 0; i < num_buckets - 1; i++, time_point += bucket_size) { + for (int i = 0; i < num_buckets - 1; i++, time_point += bucket_size) { fprintf(stdout, "Keys in range %s to %s : %lu\n", TimeToHumanString(time_point).c_str(), TimeToHumanString(time_point + bucket_size).c_str(), @@ -1547,10 +1544,10 @@ InternalDumpCommand::InternalDumpCommand( if (itr != options.end()) { delim_ = itr->second; count_delim_ = true; - // fprintf(stdout,"delim = %c\n",delim_[0]); + // fprintf(stdout,"delim = %c\n",delim_[0]); } else { count_delim_ = IsFlagPresent(flags, ARG_COUNT_DELIM); - delim_="."; + delim_ = "."; } print_stats_ = IsFlagPresent(flags, ARG_STATS); @@ -1602,8 +1599,8 @@ void InternalDumpCommand::DoCommand() { } std::string rtype1, rtype2, row, val; rtype2 = ""; - uint64_t c=0; - uint64_t s1=0,s2=0; + uint64_t c = 0; + uint64_t s1 = 0, s2 = 0; long long count = 0; for (auto& key_version : key_versions) { @@ -1618,25 +1615,24 @@ void InternalDumpCommand::DoCommand() { int k; if (count_delim_) { rtype1 = ""; - s1=0; + s1 = 0; row = ikey.Encode().ToString(); val = key_version.value; - for(k=0;row[k]!='\x01' && row[k]!='\0';k++) - s1++; - for(k=0;val[k]!='\x01' && val[k]!='\0';k++) - s1++; - for(int j=0;row[j]!=delim_[0] && row[j]!='\0' && row[j]!='\x01';j++) - rtype1+=row[j]; - if(rtype2.compare("") && rtype2.compare(rtype1)!=0) { + for (k = 0; row[k] != '\x01' && row[k] != '\0'; k++) s1++; + for (k = 0; val[k] != '\x01' && val[k] != '\0'; k++) s1++; + for (int j = 0; row[j] != delim_[0] && row[j] != '\0' && row[j] != '\x01'; + j++) + rtype1 += row[j]; + if (rtype2.compare("") && rtype2.compare(rtype1) != 0) { fprintf(stdout, "%s => count:%" PRIu64 "\tsize:%" PRIu64 "\n", rtype2.c_str(), c, s2); - c=1; - s2=s1; + c = 1; + s2 = s1; rtype2 = rtype1; } else { c++; - s2+=s1; - rtype2=rtype1; + s2 += s1; + rtype2 = rtype1; } } @@ -1649,7 +1645,7 @@ void InternalDumpCommand::DoCommand() { // Terminate if maximum number of keys have been dumped if (max_keys_ > 0 && count >= max_keys_) break; } - if(count_delim_) { + if (count_delim_) { fprintf(stdout, "%s => count:%" PRIu64 "\tsize:%" PRIu64 "\n", rtype2.c_str(), c, s2); } else { @@ -1713,7 +1709,7 @@ DBDumperCommand::DBDumperCommand( count_delim_ = true; } else { count_delim_ = IsFlagPresent(flags, ARG_COUNT_DELIM); - delim_="."; + delim_ = "."; } print_stats_ = IsFlagPresent(flags, ARG_STATS); @@ -1852,13 +1848,13 @@ void DBDumperCommand::DoDumpCommand() { int bucket_size; if (!ParseIntOption(option_map_, ARG_TTL_BUCKET, bucket_size, exec_state_) || bucket_size <= 0) { - bucket_size = time_range; // Will have just 1 bucket by default + bucket_size = time_range; // Will have just 1 bucket by default } - //cretaing variables for row count of each type + // cretaing variables for row count of each type std::string rtype1, rtype2, row, val; rtype2 = ""; - uint64_t c=0; - uint64_t s1=0,s2=0; + uint64_t c = 0; + uint64_t s1 = 0, s2 = 0; // At this point, bucket_size=0 => time_range=0 int num_buckets = (bucket_size >= time_range) @@ -1876,11 +1872,9 @@ void DBDumperCommand::DoDumpCommand() { for (; iter->Valid(); iter->Next()) { int rawtime = 0; // If end marker was specified, we stop before it - if (!null_to_ && (iter->key().ToString() >= to_)) - break; + if (!null_to_ && (iter->key().ToString() >= to_)) break; // Terminate if maximum number of keys have been dumped - if (max_keys == 0) - break; + if (max_keys == 0) break; if (is_db_ttl_) { TtlIterator* it_ttl = static_cast_with_check(iter); rawtime = it_ttl->ttl_timestamp(); @@ -1900,21 +1894,20 @@ void DBDumperCommand::DoDumpCommand() { rtype1 = ""; row = iter->key().ToString(); val = iter->value().ToString(); - s1 = row.size()+val.size(); - for(int j=0;row[j]!=delim_[0] && row[j]!='\0';j++) - rtype1+=row[j]; - if(rtype2.compare("") && rtype2.compare(rtype1)!=0) { + s1 = row.size() + val.size(); + for (int j = 0; row[j] != delim_[0] && row[j] != '\0'; j++) + rtype1 += row[j]; + if (rtype2.compare("") && rtype2.compare(rtype1) != 0) { fprintf(stdout, "%s => count:%" PRIu64 "\tsize:%" PRIu64 "\n", rtype2.c_str(), c, s2); - c=1; - s2=s1; + c = 1; + s2 = s1; rtype2 = rtype1; } else { - c++; - s2+=s1; - rtype2=rtype1; + c++; + s2 += s1; + rtype2 = rtype1; } - } if (count_only_) { @@ -1935,7 +1928,7 @@ void DBDumperCommand::DoDumpCommand() { if (num_buckets > 1 && is_db_ttl_) { PrintBucketCounts(bucket_counts, ttl_start, ttl_end, bucket_size, num_buckets); - } else if(count_delim_) { + } else if (count_delim_) { fprintf(stdout, "%s => count:%" PRIu64 "\tsize:%" PRIu64 "\n", rtype2.c_str(), c, s2); } else { @@ -1966,7 +1959,7 @@ ReduceDBLevelsCommand::ReduceDBLevelsCommand( ParseIntOption(option_map_, ARG_NEW_LEVELS, new_levels_, exec_state_); print_old_levels_ = IsFlagPresent(flags, ARG_PRINT_OLD_LEVELS); - if(new_levels_ <= 0) { + if (new_levels_ <= 0) { exec_state_ = LDBCommandExecuteResult::Failed( " Use --" + ARG_NEW_LEVELS + " to specify a new level number\n"); } @@ -1979,7 +1972,7 @@ std::vector ReduceDBLevelsCommand::PrepareArgs( ret.push_back("--" + ARG_DB + "=" + db_path); ret.push_back("--" + ARG_NEW_LEVELS + "=" + ROCKSDB_NAMESPACE::ToString(new_levels)); - if(print_old_level) { + if (print_old_level) { ret.push_back("--" + ARG_PRINT_OLD_LEVELS); } return ret; @@ -2004,8 +1997,7 @@ void ReduceDBLevelsCommand::OverrideBaseCFOptions( cf_opts->max_bytes_for_level_multiplier = 1; } -Status ReduceDBLevelsCommand::GetOldNumOfLevels(Options& opt, - int* levels) { +Status ReduceDBLevelsCommand::GetOldNumOfLevels(Options& opt, int* levels) { ImmutableDBOptions db_options(opt); EnvOptions soptions; std::shared_ptr tc( @@ -2103,9 +2095,9 @@ ChangeCompactionStyleCommand::ChangeCompactionStyleCommand( old_compaction_style_(-1), new_compaction_style_(-1) { ParseIntOption(option_map_, ARG_OLD_COMPACTION_STYLE, old_compaction_style_, - exec_state_); + exec_state_); if (old_compaction_style_ != kCompactionStyleLevel && - old_compaction_style_ != kCompactionStyleUniversal) { + old_compaction_style_ != kCompactionStyleUniversal) { exec_state_ = LDBCommandExecuteResult::Failed( "Use --" + ARG_OLD_COMPACTION_STYLE + " to specify old compaction " + "style. Check ldb help for proper compaction style value.\n"); @@ -2113,9 +2105,9 @@ ChangeCompactionStyleCommand::ChangeCompactionStyleCommand( } ParseIntOption(option_map_, ARG_NEW_COMPACTION_STYLE, new_compaction_style_, - exec_state_); + exec_state_); if (new_compaction_style_ != kCompactionStyleLevel && - new_compaction_style_ != kCompactionStyleUniversal) { + new_compaction_style_ != kCompactionStyleUniversal) { exec_state_ = LDBCommandExecuteResult::Failed( "Use --" + ARG_NEW_COMPACTION_STYLE + " to specify new compaction " + "style. Check ldb help for proper compaction style value.\n"); @@ -2444,7 +2436,6 @@ WALDumperCommand::WALDumperCommand( wal_file_ = itr->second; } - print_header_ = IsFlagPresent(flags, ARG_PRINT_HEADER); print_values_ = IsFlagPresent(flags, ARG_PRINT_VALUE); is_write_committed_ = ParseBooleanOption(options, ARG_WRITE_COMMITTED, true); @@ -2507,7 +2498,7 @@ void GetCommand::DoCommand() { Status st = db_->Get(ReadOptions(), GetCfHandle(), key_, &value); if (st.ok()) { fprintf(stdout, "%s\n", - (is_value_hex_ ? StringToHex(value) : value).c_str()); + (is_value_hex_ ? StringToHex(value) : value).c_str()); } else { std::stringstream oss; oss << "Get failed: " << st.ToString(); @@ -2746,9 +2737,9 @@ void ScanCommand::DoCommand() { TimeToHumanString(ttl_start).c_str(), TimeToHumanString(ttl_end).c_str()); } - for ( ; - it->Valid() && (!end_key_specified_ || it->key().ToString() < end_key_); - it->Next()) { + for (; + it->Valid() && (!end_key_specified_ || it->key().ToString() < end_key_); + it->Next()) { if (is_db_ttl_) { TtlIterator* it_ttl = static_cast_with_check(it); int rawtime = it_ttl->ttl_timestamp(); @@ -2946,8 +2937,9 @@ void DBQuerierCommand::Help(std::string& ret) { ret.append(DBQuerierCommand::Name()); ret.append(" [--" + ARG_TTL + "]"); ret.append("\n"); - ret.append(" Starts a REPL shell. Type help for list of available " - "commands."); + ret.append( + " Starts a REPL shell. Type help for list of available " + "commands."); ret.append("\n"); } @@ -2974,7 +2966,7 @@ void DBQuerierCommand::DoCommand() { if (pos2 == std::string::npos) { break; } - tokens.push_back(line.substr(pos, pos2-pos)); + tokens.push_back(line.substr(pos, pos2 - pos)); pos = pos2 + 1; } tokens.push_back(line.substr(pos)); @@ -3008,8 +3000,8 @@ void DBQuerierCommand::DoCommand() { key = (is_key_hex_ ? HexToString(tokens[1]) : tokens[1]); s = db_->Get(read_options, GetCfHandle(), Slice(key), &value); if (s.ok()) { - fprintf(stdout, "%s\n", PrintKeyValue(key, value, - is_key_hex_, is_value_hex_).c_str()); + fprintf(stdout, "%s\n", + PrintKeyValue(key, value, is_key_hex_, is_value_hex_).c_str()); } else { if (s.IsNotFound()) { fprintf(stdout, "Not found %s\n", tokens[1].c_str()); From 2c86b0ac2bd46b042a1b9f0418c033ed07b4c990 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Wed, 5 Jan 2022 11:01:26 -0500 Subject: [PATCH 15/39] white space changes to make Check buck targets happy --- db/db_impl/db_impl.cc | 20 ++++++-------- db/version_builder.cc | 15 ++++++---- monitoring/iostats_context.cc | 4 +-- monitoring/perf_context.cc | 28 +++++++++---------- options/options_helper.cc | 52 ++++++++++++++--------------------- options/options_helper.h | 1 - table/sst_file_dumper.cc | 5 ++-- 7 files changed, 55 insertions(+), 70 deletions(-) diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 7965f290fd6..ac8f670355d 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -1161,7 +1161,7 @@ Status DBImpl::SetDBOptions( file_options_for_compaction_ = fs_->OptimizeForCompactionTableWrite( file_options_for_compaction_, immutable_db_options_); versions_->ChangeFileOptions(mutable_db_options_); - //TODO(xiez): clarify why apply optimize for read to write options + // TODO(xiez): clarify why apply optimize for read to write options file_options_for_compaction_ = fs_->OptimizeForCompactionTableRead( file_options_for_compaction_, immutable_db_options_); file_options_for_compaction_.compaction_readahead_size = @@ -1605,7 +1605,7 @@ InternalIterator* DBImpl::NewInternalIterator(const ReadOptions& read_options, IterState* cleanup = new IterState(this, &mutex_, super_version, read_options.background_purge_on_iterator_cleanup || - immutable_db_options_.avoid_unnecessary_blocking_io); + immutable_db_options_.avoid_unnecessary_blocking_io); internal_iter->RegisterCleanup(CleanupIteratorState, cleanup, nullptr); return internal_iter; @@ -1944,8 +1944,8 @@ std::vector DBImpl::MultiGet( std::string* timestamp = timestamps ? &(*timestamps)[keys_read] : nullptr; LookupKey lkey(keys[keys_read], consistent_seqnum, read_options.timestamp); - auto cfh = - static_cast_with_check(column_family[keys_read]); + auto cfh = static_cast_with_check( + column_family[keys_read]); SequenceNumber max_covering_tombstone_seq = 0; auto mgd_iter = multiget_cf_data.find(cfh->cfd()->GetID()); assert(mgd_iter != multiget_cf_data.end()); @@ -3356,8 +3356,7 @@ SuperVersion* DBImpl::GetAndRefSuperVersion(uint32_t column_family_id) { void DBImpl::CleanupSuperVersion(SuperVersion* sv) { // Release SuperVersion if (sv->Unref()) { - bool defer_purge = - immutable_db_options().avoid_unnecessary_blocking_io; + bool defer_purge = immutable_db_options().avoid_unnecessary_blocking_io; { InstrumentedMutexLock l(&mutex_); sv->Cleanup(); @@ -4103,9 +4102,9 @@ Status DBImpl::WriteOptionsFile(bool need_mutex_lock, if (s.ok()) { s = RenameTempFileToOptionsFile(file_name); } else { - // do not accumulate failed files - // (could be hundreds with bad options code) - (void)env_->DeleteFile(file_name).ok(); + // do not accumulate failed files + // (could be hundreds with bad options code) + (void)env_->DeleteFile(file_name).ok(); } // restore lock if (!need_mutex_lock) { @@ -4913,8 +4912,7 @@ Status DBImpl::VerifyChecksumInternal(const ReadOptions& read_options, } } - bool defer_purge = - immutable_db_options().avoid_unnecessary_blocking_io; + bool defer_purge = immutable_db_options().avoid_unnecessary_blocking_io; { InstrumentedMutexLock l(&mutex_); for (auto sv : sv_list) { diff --git a/db/version_builder.cc b/db/version_builder.cc index f7f783117fb..bcf20b41d6b 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -63,7 +63,10 @@ class VersionBuilder::Rep { // kLevel0 -- NewestFirstBySeqNo // kLevelNon0 -- BySmallestKey struct FileComparator { - enum SortMethod { kLevel0 = 0, kLevelNon0 = 1, } sort_method; + enum SortMethod { + kLevel0 = 0, + kLevelNon0 = 1, + } sort_method; const InternalKeyComparator* internal_comparator; FileComparator() : internal_comparator(nullptr) {} @@ -902,7 +905,7 @@ class VersionBuilder::Rep { auto added_end = added_files.end(); while (added_iter != added_end || base_iter != base_end) { if (base_iter == base_end || - (added_iter != added_end && cmp(*added_iter, *base_iter))) { + (added_iter != added_end && cmp(*added_iter, *base_iter))) { MaybeAddFile(vstorage, level, *added_iter++); } else { MaybeAddFile(vstorage, level, *base_iter++); @@ -927,7 +930,8 @@ class VersionBuilder::Rep { bool always_load = (table_cache_capacity == TableCache::kInfiniteCapacity); size_t max_load = port::kMaxSizet; #ifndef NDEBUG - bool debug_override = true; // to enable CompactedDB related tests and some property tests + bool debug_override = + true; // to enable CompactedDB related tests and some property tests #else bool debug_override = false; #endif @@ -1008,10 +1012,11 @@ class VersionBuilder::Rep { // table cache which over time are no longer useful. The // kFilePreloadWithoutPinning option keeps #1 and disables #2. if (file_meta->table_reader_handle != nullptr) { - if (ioptions_->file_preload == kFilePreloadWithPinning || debug_override) { + if (ioptions_->file_preload == kFilePreloadWithPinning || + debug_override) { file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( file_meta->table_reader_handle); - } else { // kFilePreloadWithoutPinning + } else { // kFilePreloadWithoutPinning table_cache_->ReleaseHandle(file_meta->table_reader_handle); file_meta->table_reader_handle = nullptr; } diff --git a/monitoring/iostats_context.cc b/monitoring/iostats_context.cc index 23bf3a694f0..8335dddeaad 100644 --- a/monitoring/iostats_context.cc +++ b/monitoring/iostats_context.cc @@ -20,9 +20,7 @@ __thread IOStatsContext iostats_context; "No thread-local support. Disable iostats context with -DNIOSTATS_CONTEXT." #endif -IOStatsContext* get_iostats_context() { - return &iostats_context; -} +IOStatsContext* get_iostats_context() { return &iostats_context; } void IOStatsContext::Reset() { #ifndef NIOSTATS_CONTEXT diff --git a/monitoring/perf_context.cc b/monitoring/perf_context.cc index d45d84fb6e3..2151536a26b 100644 --- a/monitoring/perf_context.cc +++ b/monitoring/perf_context.cc @@ -23,12 +23,11 @@ thread_local PerfContext perf_context; #error "No thread-local support. Disable perf context with -DNPERF_CONTEXT." #endif -PerfContext* get_perf_context() { - return &perf_context; -} +PerfContext* get_perf_context() { return &perf_context; } PerfContext::~PerfContext() { -#if !defined(NPERF_CONTEXT) && defined(ROCKSDB_SUPPORT_THREAD_LOCAL) && !defined(OS_SOLARIS) +#if !defined(NPERF_CONTEXT) && defined(ROCKSDB_SUPPORT_THREAD_LOCAL) && \ + !defined(OS_SOLARIS) ClearPerLevelPerfContext(); #endif } @@ -422,15 +421,14 @@ void PerfContext::Reset() { ss << #counter << " = " << counter << ", "; \ } -#define PERF_CONTEXT_BY_LEVEL_OUTPUT_ONE_COUNTER(counter) \ - if (per_level_perf_context_enabled && \ - level_to_perf_context) { \ - ss << #counter << " = "; \ - for (auto& kv : *level_to_perf_context) { \ - if (!exclude_zero_counters || (kv.second.counter > 0)) { \ - ss << kv.second.counter << "@level" << kv.first << ", "; \ - } \ - } \ +#define PERF_CONTEXT_BY_LEVEL_OUTPUT_ONE_COUNTER(counter) \ + if (per_level_perf_context_enabled && level_to_perf_context) { \ + ss << #counter << " = "; \ + for (auto& kv : *level_to_perf_context) { \ + if (!exclude_zero_counters || (kv.second.counter > 0)) { \ + ss << kv.second.counter << "@level" << kv.first << ", "; \ + } \ + } \ } void PerfContextByLevel::Reset() { @@ -546,11 +544,11 @@ void PerfContext::EnablePerLevelPerfContext() { per_level_perf_context_enabled = true; } -void PerfContext::DisablePerLevelPerfContext(){ +void PerfContext::DisablePerLevelPerfContext() { per_level_perf_context_enabled = false; } -void PerfContext::ClearPerLevelPerfContext(){ +void PerfContext::ClearPerLevelPerfContext() { if (level_to_perf_context != nullptr) { level_to_perf_context->clear(); delete level_to_perf_context; diff --git a/options/options_helper.cc b/options/options_helper.cc index 958c8387607..3b8879d57bb 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -165,10 +165,8 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options, immutable_db_options.avoid_flush_during_recovery; options.avoid_flush_during_shutdown = mutable_db_options.avoid_flush_during_shutdown; - options.allow_ingest_behind = - immutable_db_options.allow_ingest_behind; - options.preserve_deletes = - immutable_db_options.preserve_deletes; + options.allow_ingest_behind = immutable_db_options.allow_ingest_behind; + options.preserve_deletes = immutable_db_options.preserve_deletes; options.two_write_queues = immutable_db_options.two_write_queues; options.manual_wal_flush = immutable_db_options.manual_wal_flush; options.atomic_flush = immutable_db_options.atomic_flush; @@ -515,13 +513,11 @@ bool SerializeSingleOptionHelper(const void* opt_address, case OptionType::kInt32T: *value = ToString(*(static_cast(opt_address))); break; - case OptionType::kInt64T: - { - int64_t v; - GetUnaligned(static_cast(opt_address), &v); - *value = ToString(v); - } - break; + case OptionType::kInt64T: { + int64_t v; + GetUnaligned(static_cast(opt_address), &v); + *value = ToString(v); + } break; case OptionType::kUInt: *value = ToString(*(static_cast(opt_address))); break; @@ -531,20 +527,16 @@ bool SerializeSingleOptionHelper(const void* opt_address, case OptionType::kUInt32T: *value = ToString(*(static_cast(opt_address))); break; - case OptionType::kUInt64T: - { - uint64_t v; - GetUnaligned(static_cast(opt_address), &v); - *value = ToString(v); - } - break; - case OptionType::kSizeT: - { - size_t v; - GetUnaligned(static_cast(opt_address), &v); - *value = ToString(v); - } - break; + case OptionType::kUInt64T: { + uint64_t v; + GetUnaligned(static_cast(opt_address), &v); + *value = ToString(v); + } break; + case OptionType::kSizeT: { + size_t v; + GetUnaligned(static_cast(opt_address), &v); + *value = ToString(v); + } break; case OptionType::kDouble: *value = ToString(*(static_cast(opt_address))); break; @@ -650,7 +642,6 @@ Status ConfigureFromMap( return s; } - Status StringToMap(const std::string& opts_str, std::unordered_map* opts_map) { assert(opts_map); @@ -691,7 +682,6 @@ Status StringToMap(const std::string& opts_str, return Status::OK(); } - Status GetStringFromDBOptions(std::string* opt_string, const DBOptions& db_options, const std::string& delimiter) { @@ -709,7 +699,6 @@ Status GetStringFromDBOptions(const ConfigOptions& config_options, return config->GetOptionString(config_options, opt_string); } - Status GetStringFromColumnFamilyOptions(std::string* opt_string, const ColumnFamilyOptions& cf_options, const std::string& delimiter) { @@ -770,10 +759,9 @@ Status GetColumnFamilyOptionsFromMap( } } -Status GetColumnFamilyOptionsFromString( - const ColumnFamilyOptions& base_options, - const std::string& opts_str, - ColumnFamilyOptions* new_options) { +Status GetColumnFamilyOptionsFromString(const ColumnFamilyOptions& base_options, + const std::string& opts_str, + ColumnFamilyOptions* new_options) { ConfigOptions config_options; config_options.input_strings_escaped = false; config_options.ignore_unknown_options = false; diff --git a/options/options_helper.h b/options/options_helper.h index 6692ffaad78..469dcacc6da 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -54,7 +54,6 @@ std::unique_ptr CFOptionsAsConfigurable( const ColumnFamilyOptions& opts, const std::unordered_map* opt_map = nullptr); - bool ParseSliceTransform( const std::string& value, std::shared_ptr* slice_transform); diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc index d2ade5bfeaf..5c163322a6d 100644 --- a/table/sst_file_dumper.cc +++ b/table/sst_file_dumper.cc @@ -205,9 +205,8 @@ Status SstFileDumper::CalculateCompressedTableSize( table_options.block_size = block_size; BlockBasedTableFactory block_based_tf(table_options); std::unique_ptr table_builder; - table_builder.reset(block_based_tf.NewTableBuilder( - tb_options, - dest_writer.get())); + table_builder.reset( + block_based_tf.NewTableBuilder(tb_options, dest_writer.get())); std::unique_ptr iter(table_reader_->NewIterator( read_options_, moptions_.prefix_extractor.get(), /*arena=*/nullptr, /*skip_filters=*/false, TableReaderCaller::kSSTDumpTool)); From 306ce12934f74326c2290f7c80079aa9384e13ec Mon Sep 17 00:00:00 2001 From: matthewvon Date: Wed, 5 Jan 2022 11:06:33 -0500 Subject: [PATCH 16/39] white space changes to make Check buck targets happy --- port/win/env_win.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/port/win/env_win.cc b/port/win/env_win.cc index cc337c1f8c6..c4acc5e4171 100644 --- a/port/win/env_win.cc +++ b/port/win/env_win.cc @@ -9,6 +9,8 @@ #if defined(OS_WIN) +#include "port/win/env_win.h" + #include // _rmdir, _mkdir, _getcwd #include #include // _access @@ -27,7 +29,6 @@ #include "monitoring/thread_status_util.h" #include "port/port.h" #include "port/port_dirent.h" -#include "port/win/env_win.h" #include "port/win/io_win.h" #include "port/win/win_logger.h" #include "port/win/win_thread.h" From 911ecec1015184dbba8a6d516c6af3b4436a67bc Mon Sep 17 00:00:00 2001 From: matthewvon Date: Thu, 6 Jan 2022 15:00:29 -0500 Subject: [PATCH 17/39] add FilePreload options --- db/db_test_util.cc | 8 ++++++++ db/db_test_util.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/db/db_test_util.cc b/db/db_test_util.cc index ef10ec47998..e03f2c7f910 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -558,6 +558,14 @@ Options DBTestBase::GetOptions( options.unordered_write = false; break; } + case kFilePreloadWithoutPinning: { + options.file_preload = FilePreload::kFilePreloadWithoutPinning; + break; + } + case kFilePreloadDisabled: { + options.file_preload = FilePreload::kFilePreloadDisabled; + break; + } default: break; diff --git a/db/db_test_util.h b/db/db_test_util.h index 3ffa5a5d574..8040c1e398f 100644 --- a/db/db_test_util.h +++ b/db/db_test_util.h @@ -941,6 +941,8 @@ class DBTestBase : public testing::Test { kUniversalSubcompactions, kxxHash64Checksum, kUnorderedWrite, + kFilePreloadWithoutPinning, + kFilePreloadDisabled, // This must be the last line kEnd, }; From 80c0994c4f7098a39672c0495152dff711b37669 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Thu, 6 Jan 2022 15:01:34 -0500 Subject: [PATCH 18/39] remove unnecessary debug_override flag. handle edge case of one file loading when kFilePreloadDisabled set --- db/version_builder.cc | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index bcf20b41d6b..8efe43c0365 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -929,12 +929,7 @@ class VersionBuilder::Rep { size_t table_cache_capacity = table_cache_->get_cache()->GetCapacity(); bool always_load = (table_cache_capacity == TableCache::kInfiniteCapacity); size_t max_load = port::kMaxSizet; -#ifndef NDEBUG - bool debug_override = - true; // to enable CompactedDB related tests and some property tests -#else - bool debug_override = false; -#endif + if (FilePreload::kFilePreloadDisabled == ioptions_->file_preload) { max_load = 0; always_load = true; @@ -1012,8 +1007,7 @@ class VersionBuilder::Rep { // table cache which over time are no longer useful. The // kFilePreloadWithoutPinning option keeps #1 and disables #2. if (file_meta->table_reader_handle != nullptr) { - if (ioptions_->file_preload == kFilePreloadWithPinning || - debug_override) { + if (ioptions_->file_preload == kFilePreloadWithPinning) { file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( file_meta->table_reader_handle); } else { // kFilePreloadWithoutPinning @@ -1024,19 +1018,22 @@ class VersionBuilder::Rep { } }); - std::vector threads; - for (int i = 1; i < max_threads; i++) { - threads.emplace_back(load_handlers_func); - } - load_handlers_func(); - for (auto& t : threads) { - t.join(); - } Status ret; - for (const auto& s : statuses) { - if (!s.ok()) { - if (ret.ok()) { - ret = s; + if (0 < max_load) { + std::vector threads; + for (int i = 1; i < max_threads; i++) { + threads.emplace_back(load_handlers_func); + } + load_handlers_func(); + for (auto& t : threads) { + t.join(); + } + + for (const auto& s : statuses) { + if (!s.ok()) { + if (ret.ok()) { + ret = s; + } } } } From a734cd7d00f2baa7285e6d8d8fc1193705843ae0 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Thu, 6 Jan 2022 15:02:14 -0500 Subject: [PATCH 19/39] add test for new file_preload flag --- db/version_builder_test.cc | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 26b473f0fe6..08de71dfcdf 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -9,6 +9,7 @@ #include #include +#include "db/db_test_util.h" #include "db/version_edit.h" #include "db/version_set.h" #include "test_util/testharness.h" @@ -1543,6 +1544,47 @@ TEST_F(VersionBuilderTest, EstimatedActiveKeys) { (kEntriesPerFile - 2 * kDeletionsPerFile) * kNumFiles); } +class FilePreloadTest : public DBTestBase { + public: + FilePreloadTest() : DBTestBase("/file_preload_test", /*env_do_fsync=*/true) {} +}; + +TEST_F(FilePreloadTest, PreloadOptions) { + // create a DB with 3 files + // the day. + ASSERT_OK(Put("key", "val")); + ASSERT_OK(Flush()); + ASSERT_OK(Put("key2", "val2")); + ASSERT_OK(Flush()); + ASSERT_OK(Put("key3", "val3")); + ASSERT_OK(Flush()); + + DBImpl* db_impl = dbfull(); + Cache* table_cache = db_impl->TEST_table_cache(); + + ASSERT_EQ(table_cache->GetUsage(), 3); + table_cache->EraseUnRefEntries(); + ASSERT_EQ(table_cache->GetUsage(), 3); + + Options new_options = GetOptions(kFilePreloadWithoutPinning); + Reopen(new_options); + db_impl = dbfull(); + table_cache = db_impl->TEST_table_cache(); + + ASSERT_EQ(table_cache->GetUsage(), 3); + table_cache->EraseUnRefEntries(); + ASSERT_EQ(table_cache->GetUsage(), 0); + + new_options = GetOptions(kFilePreloadDisabled); + Reopen(new_options); + db_impl = dbfull(); + table_cache = db_impl->TEST_table_cache(); + + ASSERT_EQ(table_cache->GetUsage(), 0); + table_cache->EraseUnRefEntries(); + ASSERT_EQ(table_cache->GetUsage(), 0); +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { From 970503663b660e7f30d42b6a73bb6351839c6da0 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 13:49:03 -0500 Subject: [PATCH 20/39] fix unrelated bug in options parser so that options_util_test will execute. --- include/rocksdb/utilities/options_type.h | 2 ++ options/configurable.cc | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/rocksdb/utilities/options_type.h b/include/rocksdb/utilities/options_type.h index 23186d2532a..6bf0cdc0076 100644 --- a/include/rocksdb/utilities/options_type.h +++ b/include/rocksdb/utilities/options_type.h @@ -593,6 +593,8 @@ class OptionTypeInfo { bool IsStruct() const { return (type_ == OptionType::kStruct); } + bool IsVector() const { return (type_ == OptionType::kVector); } + bool IsConfigurable() const { return (type_ == OptionType::kConfigurable || type_ == OptionType::kCustomizable); diff --git a/options/configurable.cc b/options/configurable.cc index 5f916078a19..48a79393bff 100644 --- a/options/configurable.cc +++ b/options/configurable.cc @@ -608,7 +608,7 @@ Status ConfigurableHelper::SerializeOptions(const ConfigOptions& config_options, } if (!s.ok()) { return s; - } else if (!value.empty()) { + } else if (opt_info.IsVector() || !value.empty()) { // = result->append(prefix + opt_name + "=" + value + config_options.delimiter); From 99aea0c9779198e208b62a6ba26f417f8b916fb8 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 14:11:44 -0500 Subject: [PATCH 21/39] clean up shadowing error from CI --- db/db_test_util.cc | 4 ++-- db/db_test_util.h | 4 ++-- db/version_builder_test.cc | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/db/db_test_util.cc b/db/db_test_util.cc index 74b153a0079..f728ac84112 100644 --- a/db/db_test_util.cc +++ b/db/db_test_util.cc @@ -565,11 +565,11 @@ Options DBTestBase::GetOptions( options.unordered_write = false; break; } - case kFilePreloadWithoutPinning: { + case kPreloadWithoutPinning: { options.file_preload = FilePreload::kFilePreloadWithoutPinning; break; } - case kFilePreloadDisabled: { + case kPreloadDisabled: { options.file_preload = FilePreload::kFilePreloadDisabled; break; } diff --git a/db/db_test_util.h b/db/db_test_util.h index aa7d3b2a309..280c26f253b 100644 --- a/db/db_test_util.h +++ b/db/db_test_util.h @@ -874,8 +874,8 @@ class DBTestBase : public testing::Test { kPartitionedFilterWithNewTableReaderForCompactions, kUniversalSubcompactions, kUnorderedWrite, - kFilePreloadWithoutPinning, - kFilePreloadDisabled, + kPreloadWithoutPinning, + kPreloadDisabled, // This must be the last line kEnd, }; diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index b94e079772d..6b7a99410e4 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -1678,7 +1678,7 @@ TEST_F(FilePreloadTest, PreloadOptions) { table_cache->EraseUnRefEntries(); ASSERT_EQ(table_cache->GetUsage(), 3); - Options new_options = GetOptions(kFilePreloadWithoutPinning); + Options new_options = GetOptions(kPreloadWithoutPinning); Reopen(new_options); db_impl = dbfull(); table_cache = db_impl->TEST_table_cache(); @@ -1687,7 +1687,7 @@ TEST_F(FilePreloadTest, PreloadOptions) { table_cache->EraseUnRefEntries(); ASSERT_EQ(table_cache->GetUsage(), 0); - new_options = GetOptions(kFilePreloadDisabled); + new_options = GetOptions(kPreloadDisabled); Reopen(new_options); db_impl = dbfull(); table_cache = db_impl->TEST_table_cache(); From ab23d652637159a9b9fb904c194382f4592b25d4 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 14:28:50 -0500 Subject: [PATCH 22/39] narrow when zero length vector prints --- options/configurable.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options/configurable.cc b/options/configurable.cc index 48a79393bff..05468430e5a 100644 --- a/options/configurable.cc +++ b/options/configurable.cc @@ -608,7 +608,7 @@ Status ConfigurableHelper::SerializeOptions(const ConfigOptions& config_options, } if (!s.ok()) { return s; - } else if (opt_info.IsVector() || !value.empty()) { + } else if (!value.empty() || (opt_info.IsVector() && opt_info.IsMutable())) { // = result->append(prefix + opt_name + "=" + value + config_options.delimiter); From 138d77e67cfe9db55cc964eecb10f3e0f6f31b55 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 14:47:24 -0500 Subject: [PATCH 23/39] make check buck targets happy --- options/configurable.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/options/configurable.cc b/options/configurable.cc index 05468430e5a..3607cfb1f0c 100644 --- a/options/configurable.cc +++ b/options/configurable.cc @@ -608,7 +608,8 @@ Status ConfigurableHelper::SerializeOptions(const ConfigOptions& config_options, } if (!s.ok()) { return s; - } else if (!value.empty() || (opt_info.IsVector() && opt_info.IsMutable())) { + } else if (!value.empty() || + (opt_info.IsVector() && opt_info.IsMutable())) { // = result->append(prefix + opt_name + "=" + value + config_options.delimiter); From be3a403ec29d73bedf177b5244fafbbe7fc1426b Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 14:53:04 -0500 Subject: [PATCH 24/39] check the unchecked status variable --- db/version_builder.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/version_builder.cc b/db/version_builder.cc index 7f52f570eda..52c0f6f971b 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -1261,6 +1261,8 @@ class VersionBuilder::Rep { } } } + } else { + (void) ret.ok(); // for testing } return ret; } From 3a7d20fc3a3ddf0c335bad628586c0e43664f326 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 16:36:58 -0500 Subject: [PATCH 25/39] clean up what code runs if file preload is disabled ... fixes Status checking errors --- db/version_builder.cc | 103 +++++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 52 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index 52c0f6f971b..c3aa67e6d67 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -1186,65 +1186,65 @@ class VersionBuilder::Rep { } } - // - std::vector> files_meta; - std::vector statuses; - for (int level = 0; level < num_levels_; level++) { - for (auto& file_meta_pair : levels_[level].added_files) { - auto* file_meta = file_meta_pair.second; - // If the file has been opened before, just skip it. - if (!file_meta->table_reader_handle) { - files_meta.emplace_back(file_meta, level); - statuses.emplace_back(Status::OK()); + Status ret; + if (0 < max_load) { + // + std::vector> files_meta; + std::vector statuses; + for (int level = 0; level < num_levels_; level++) { + for (auto& file_meta_pair : levels_[level].added_files) { + auto* file_meta = file_meta_pair.second; + // If the file has been opened before, just skip it. + if (!file_meta->table_reader_handle) { + files_meta.emplace_back(file_meta, level); + statuses.emplace_back(Status::OK()); + } + if (files_meta.size() >= max_load) { + break; + } } if (files_meta.size() >= max_load) { break; } } - if (files_meta.size() >= max_load) { - break; - } - } - std::atomic next_file_meta_idx(0); - std::function load_handlers_func([&]() { - while (true) { - size_t file_idx = next_file_meta_idx.fetch_add(1); - if (file_idx >= files_meta.size()) { - break; - } + std::atomic next_file_meta_idx(0); + std::function load_handlers_func([&]() { + while (true) { + size_t file_idx = next_file_meta_idx.fetch_add(1); + if (file_idx >= files_meta.size()) { + break; + } - auto* file_meta = files_meta[file_idx].first; - int level = files_meta[file_idx].second; - statuses[file_idx] = table_cache_->FindTable( - ReadOptions(), file_options_, - *(base_vstorage_->InternalComparator()), file_meta->fd, - &file_meta->table_reader_handle, prefix_extractor, false /*no_io */, - true /* record_read_stats */, - internal_stats->GetFileReadHist(level), false, level, - prefetch_index_and_filter_in_cache, max_file_size_for_l0_meta_pin, - file_meta->temperature); - - // The code is attempting two things: - // 1. preload / warm the table cache with new file objects - // 2. create higher performance via a cache lookup avoidance - // The issue is that number 2 creates permanent objects in the - // table cache which over time are no longer useful. The - // kFilePreloadWithoutPinning option keeps #1 and disables #2. - if (file_meta->table_reader_handle != nullptr) { - if (ioptions_->file_preload == kFilePreloadWithPinning) { - file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( - file_meta->table_reader_handle); - } else { // kFilePreloadWithoutPinning - table_cache_->ReleaseHandle(file_meta->table_reader_handle); - file_meta->table_reader_handle = nullptr; + auto* file_meta = files_meta[file_idx].first; + int level = files_meta[file_idx].second; + statuses[file_idx] = table_cache_->FindTable( + ReadOptions(), file_options_, + *(base_vstorage_->InternalComparator()), file_meta->fd, + &file_meta->table_reader_handle, prefix_extractor, false /*no_io */, + true /* record_read_stats */, + internal_stats->GetFileReadHist(level), false, level, + prefetch_index_and_filter_in_cache, max_file_size_for_l0_meta_pin, + file_meta->temperature); + + // The code is attempting two things: + // 1. preload / warm the table cache with new file objects + // 2. create higher performance via a cache lookup avoidance + // The issue is that number 2 creates permanent objects in the + // table cache which over time are no longer useful. The + // kFilePreloadWithoutPinning option keeps #1 and disables #2. + if (file_meta->table_reader_handle != nullptr) { + if (ioptions_->file_preload == kFilePreloadWithPinning) { + file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( + file_meta->table_reader_handle); + } else { // kFilePreloadWithoutPinning + table_cache_->ReleaseHandle(file_meta->table_reader_handle); + file_meta->table_reader_handle = nullptr; + } + } } - } - } - }); + }); - Status ret; - if (0 < max_load) { std::vector threads; for (int i = 1; i < max_threads; i++) { threads.emplace_back(load_handlers_func); @@ -1261,9 +1261,8 @@ class VersionBuilder::Rep { } } } - } else { - (void) ret.ok(); // for testing } + return ret; } }; From dfc5da77bed927189a735ad3c4a663dc2216f6ca Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 16:40:21 -0500 Subject: [PATCH 26/39] make check buck targets happy --- db/version_builder.cc | 63 ++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index c3aa67e6d67..ef995148a8d 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -1210,40 +1210,41 @@ class VersionBuilder::Rep { std::atomic next_file_meta_idx(0); std::function load_handlers_func([&]() { - while (true) { - size_t file_idx = next_file_meta_idx.fetch_add(1); - if (file_idx >= files_meta.size()) { - break; - } + while (true) { + size_t file_idx = next_file_meta_idx.fetch_add(1); + if (file_idx >= files_meta.size()) { + break; + } - auto* file_meta = files_meta[file_idx].first; - int level = files_meta[file_idx].second; - statuses[file_idx] = table_cache_->FindTable( - ReadOptions(), file_options_, - *(base_vstorage_->InternalComparator()), file_meta->fd, - &file_meta->table_reader_handle, prefix_extractor, false /*no_io */, - true /* record_read_stats */, - internal_stats->GetFileReadHist(level), false, level, - prefetch_index_and_filter_in_cache, max_file_size_for_l0_meta_pin, - file_meta->temperature); - - // The code is attempting two things: - // 1. preload / warm the table cache with new file objects - // 2. create higher performance via a cache lookup avoidance - // The issue is that number 2 creates permanent objects in the - // table cache which over time are no longer useful. The - // kFilePreloadWithoutPinning option keeps #1 and disables #2. - if (file_meta->table_reader_handle != nullptr) { - if (ioptions_->file_preload == kFilePreloadWithPinning) { - file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( - file_meta->table_reader_handle); - } else { // kFilePreloadWithoutPinning - table_cache_->ReleaseHandle(file_meta->table_reader_handle); - file_meta->table_reader_handle = nullptr; - } + auto* file_meta = files_meta[file_idx].first; + int level = files_meta[file_idx].second; + statuses[file_idx] = table_cache_->FindTable( + ReadOptions(), file_options_, + *(base_vstorage_->InternalComparator()), file_meta->fd, + &file_meta->table_reader_handle, prefix_extractor, + false /*no_io */, true /* record_read_stats */, + internal_stats->GetFileReadHist(level), false, level, + prefetch_index_and_filter_in_cache, max_file_size_for_l0_meta_pin, + file_meta->temperature); + + // The code is attempting two things: + // 1. preload / warm the table cache with new file objects + // 2. create higher performance via a cache lookup avoidance + // The issue is that number 2 creates permanent objects in the + // table cache which over time are no longer useful. The + // kFilePreloadWithoutPinning option keeps #1 and disables #2. + if (file_meta->table_reader_handle != nullptr) { + if (ioptions_->file_preload == kFilePreloadWithPinning) { + file_meta->fd.table_reader = + table_cache_->GetTableReaderFromHandle( + file_meta->table_reader_handle); + } else { // kFilePreloadWithoutPinning + table_cache_->ReleaseHandle(file_meta->table_reader_handle); + file_meta->table_reader_handle = nullptr; } } - }); + } + }); std::vector threads; for (int i = 1; i < max_threads; i++) { From 5f06aa3e31272cc5fb1a5ad5b8f4a645a8d25ea7 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 7 Jan 2022 21:43:30 -0500 Subject: [PATCH 27/39] add test showing corruption will pass --- db/version_builder_test.cc | 55 +++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 6b7a99410e4..6599c872cda 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -1661,7 +1661,7 @@ class FilePreloadTest : public DBTestBase { FilePreloadTest() : DBTestBase("/file_preload_test", /*env_do_fsync=*/true) {} }; -TEST_F(FilePreloadTest, PreloadOptions) { +TEST_F(FilePreloadTest, PreloadCaching) { // create a DB with 3 files // the day. ASSERT_OK(Put("key", "val")); @@ -1674,27 +1674,68 @@ TEST_F(FilePreloadTest, PreloadOptions) { DBImpl* db_impl = dbfull(); Cache* table_cache = db_impl->TEST_table_cache(); - ASSERT_EQ(table_cache->GetUsage(), 3); + ASSERT_EQ(table_cache->GetUsage(), 3) << "preload: failed"; table_cache->EraseUnRefEntries(); - ASSERT_EQ(table_cache->GetUsage(), 3); + ASSERT_EQ(table_cache->GetUsage(), 3) << "pinning: failed"; Options new_options = GetOptions(kPreloadWithoutPinning); Reopen(new_options); db_impl = dbfull(); table_cache = db_impl->TEST_table_cache(); - ASSERT_EQ(table_cache->GetUsage(), 3); + ASSERT_EQ(table_cache->GetUsage(), 3) << "preload: failed"; table_cache->EraseUnRefEntries(); - ASSERT_EQ(table_cache->GetUsage(), 0); + ASSERT_EQ(table_cache->GetUsage(), 0) << "pinning: should not happen"; new_options = GetOptions(kPreloadDisabled); Reopen(new_options); db_impl = dbfull(); table_cache = db_impl->TEST_table_cache(); - ASSERT_EQ(table_cache->GetUsage(), 0); + ASSERT_EQ(table_cache->GetUsage(), 0) << "preload: should not happen"; table_cache->EraseUnRefEntries(); - ASSERT_EQ(table_cache->GetUsage(), 0); + ASSERT_EQ(table_cache->GetUsage(), 0) << "pinning: should not happen"; +} + +TEST_F(FilePreloadTest, PreloadCorruption) { + // create a DB with 3 files + // the day. + ASSERT_OK(Put("key", "val")); + ASSERT_OK(Flush()); + ASSERT_OK(Put("key2", "val2")); + ASSERT_OK(Flush()); + ASSERT_OK(Put("key3", "val3")); + ASSERT_OK(Flush()); + + DBImpl* db_impl = dbfull(); + Cache* table_cache = db_impl->TEST_table_cache(); + + ASSERT_EQ(table_cache->GetUsage(), 3); + table_cache->EraseUnRefEntries(); + ASSERT_EQ(table_cache->GetUsage(), 3); + + Options new_options = GetOptions(kDefault); + ASSERT_TRUE(TryReopen(new_options).ok()); + db_impl = dbfull(); + table_cache = db_impl->TEST_table_cache(); + + ASSERT_EQ(table_cache->GetUsage(), 3); + table_cache->EraseUnRefEntries(); + ASSERT_EQ(table_cache->GetUsage(), 3); + + // find name of txn file + rocksdb::ColumnFamilyMetaData meta; + db_->GetColumnFamilyMetaData(&meta); + // name starts with slash + std::string fail_file = meta.levels[0].files[0].db_path + meta.levels[0].files[0].name; + + ASSERT_TRUE(WriteStringToFile(db_->GetEnv(), ":#)", fail_file, true).ok()); + ASSERT_FALSE(TryReopen(new_options).ok()); + + new_options = GetOptions(kPreloadDisabled); + new_options.paranoid_checks = false; + Status ns = TryReopen(new_options); + ASSERT_TRUE(TryReopen(new_options).ok()); } } // namespace ROCKSDB_NAMESPACE From e5bc590cc141229efde5ccce6a2745d893efa2b8 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Sat, 8 Jan 2022 10:59:45 -0500 Subject: [PATCH 28/39] Make PreloadCorruption test valid with changing paranoid_checks flag --- db/version_builder_test.cc | 43 ++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 6599c872cda..9b4b81fc578 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -1624,10 +1624,10 @@ TEST_F(VersionBuilderTest, CheckConsistencyForFileDeletedTwice) { ASSERT_OK(version_builder.SaveTo(&new_vstorage)); VersionBuilder version_builder2(env_options, &ioptions_, table_cache, - &new_vstorage, version_set); + &new_vstorage, version_set); VersionStorageInfo new_vstorage2(&icmp_, ucmp_, options_.num_levels, - kCompactionStyleLevel, nullptr, - true /* force_consistency_checks */); + kCompactionStyleLevel, nullptr, + true /* force_consistency_checks */); ASSERT_NOK(version_builder2.Apply(&version_edit)); UnrefFilesInVersion(&new_vstorage); @@ -1644,10 +1644,8 @@ TEST_F(VersionBuilderTest, EstimatedActiveKeys) { for (uint32_t i = 0; i < kNumFiles; ++i) { Add(static_cast(i / kFilesPerLevel), i + 1, ToString((i + 100) * 1000).c_str(), - ToString((i + 100) * 1000 + 999).c_str(), - 100U, 0, 100, 100, - kEntriesPerFile, kDeletionsPerFile, - (i < kTotalSamples)); + ToString((i + 100) * 1000 + 999).c_str(), 100U, 0, 100, 100, + kEntriesPerFile, kDeletionsPerFile, (i < kTotalSamples)); } // minus 2X for the number of deletion entries because: // 1x for deletion entry does not count as a data entry. @@ -1674,27 +1672,29 @@ TEST_F(FilePreloadTest, PreloadCaching) { DBImpl* db_impl = dbfull(); Cache* table_cache = db_impl->TEST_table_cache(); - ASSERT_EQ(table_cache->GetUsage(), 3) << "preload: failed"; + ASSERT_EQ(table_cache->GetUsage(), 3) << "with preload: failed"; table_cache->EraseUnRefEntries(); - ASSERT_EQ(table_cache->GetUsage(), 3) << "pinning: failed"; + ASSERT_EQ(table_cache->GetUsage(), 3) << "with pinning: failed"; Options new_options = GetOptions(kPreloadWithoutPinning); Reopen(new_options); db_impl = dbfull(); table_cache = db_impl->TEST_table_cache(); - ASSERT_EQ(table_cache->GetUsage(), 3) << "preload: failed"; + ASSERT_EQ(table_cache->GetUsage(), 3) << "without preload: failed"; table_cache->EraseUnRefEntries(); - ASSERT_EQ(table_cache->GetUsage(), 0) << "pinning: should not happen"; + ASSERT_EQ(table_cache->GetUsage(), 0) << "without pinning: should not happen"; new_options = GetOptions(kPreloadDisabled); Reopen(new_options); db_impl = dbfull(); table_cache = db_impl->TEST_table_cache(); - ASSERT_EQ(table_cache->GetUsage(), 0) << "preload: should not happen"; + ASSERT_EQ(table_cache->GetUsage(), 0) + << "disabled preload: should not happen"; table_cache->EraseUnRefEntries(); - ASSERT_EQ(table_cache->GetUsage(), 0) << "pinning: should not happen"; + ASSERT_EQ(table_cache->GetUsage(), 0) + << "disabled pinning: should not happen"; } TEST_F(FilePreloadTest, PreloadCorruption) { @@ -1723,19 +1723,22 @@ TEST_F(FilePreloadTest, PreloadCorruption) { table_cache->EraseUnRefEntries(); ASSERT_EQ(table_cache->GetUsage(), 3); - // find name of txn file + // find name of txn file + // must corrupt file of same size to bypass paranoid_checks fail rocksdb::ColumnFamilyMetaData meta; db_->GetColumnFamilyMetaData(&meta); // name starts with slash - std::string fail_file = meta.levels[0].files[0].db_path + meta.levels[0].files[0].name; + std::string fail_file = + meta.levels[0].files[0].db_path + meta.levels[0].files[0].name; + std::string garbage(meta.levels[0].files[0].size, '@'); + ASSERT_TRUE(WriteStringToFile(db_->GetEnv(), garbage, fail_file, true).ok()); - ASSERT_TRUE(WriteStringToFile(db_->GetEnv(), ":#)", fail_file, true).ok()); - ASSERT_FALSE(TryReopen(new_options).ok()); + ASSERT_FALSE(TryReopen(new_options).ok()) + << "reopen should fail with corrupted .sst"; new_options = GetOptions(kPreloadDisabled); - new_options.paranoid_checks = false; - Status ns = TryReopen(new_options); - ASSERT_TRUE(TryReopen(new_options).ok()); + ASSERT_TRUE(TryReopen(new_options).ok()) + << "reopen should fail with preload disabled"; } } // namespace ROCKSDB_NAMESPACE From 4c2acb58418354e0cb7639ade6b5386f61ded88f Mon Sep 17 00:00:00 2001 From: matthewvon Date: Sat, 8 Jan 2022 11:27:04 -0500 Subject: [PATCH 29/39] disable PreloadCorruption test within ROCKSDB_LITE builds --- db/version_builder_test.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 9b4b81fc578..18a280e9e4d 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -1697,6 +1697,9 @@ TEST_F(FilePreloadTest, PreloadCaching) { << "disabled pinning: should not happen"; } +#ifndef ROCKSDB_LITE +// lite does not support GetColumnFamilyMetaData() + TEST_F(FilePreloadTest, PreloadCorruption) { // create a DB with 3 files // the day. @@ -1740,7 +1743,7 @@ TEST_F(FilePreloadTest, PreloadCorruption) { ASSERT_TRUE(TryReopen(new_options).ok()) << "reopen should fail with preload disabled"; } - +#endif } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { From 285bd3910e49535ba37efb2260ae356f825f8612 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Sat, 8 Jan 2022 11:38:05 -0500 Subject: [PATCH 30/39] removed explicit rocksdb:: namespace reference ... for CircleCI happiness --- db/version_builder_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 18a280e9e4d..9c6368a5dcb 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -1728,7 +1728,7 @@ TEST_F(FilePreloadTest, PreloadCorruption) { // find name of txn file // must corrupt file of same size to bypass paranoid_checks fail - rocksdb::ColumnFamilyMetaData meta; + ColumnFamilyMetaData meta; db_->GetColumnFamilyMetaData(&meta); // name starts with slash std::string fail_file = From 01fbbf734f67fab1ba8c2f4d6e4947bc1c424199 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 11 Jan 2022 14:07:58 -0500 Subject: [PATCH 31/39] address PR comments --- db/db_impl/db_impl.cc | 2 +- db/version_builder.cc | 2 +- db/version_builder_test.cc | 10 ++++------ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 2d1262263da..b73fa0c4c57 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -4242,7 +4242,7 @@ Status DBImpl::WriteOptionsFile(bool need_mutex_lock, } else { // do not accumulate failed files // (could be hundreds with bad options code) - (void)env_->DeleteFile(file_name).ok(); + env_->DeleteFile(file_name).PermitUncheckedError(); } // restore lock if (!need_mutex_lock) { diff --git a/db/version_builder.cc b/db/version_builder.cc index ef995148a8d..e54dd909062 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -1187,7 +1187,7 @@ class VersionBuilder::Rep { } Status ret; - if (0 < max_load) { + if (max_load > 0) { // std::vector> files_meta; std::vector statuses; diff --git a/db/version_builder_test.cc b/db/version_builder_test.cc index 9c6368a5dcb..2a1e983aaab 100644 --- a/db/version_builder_test.cc +++ b/db/version_builder_test.cc @@ -1661,7 +1661,6 @@ class FilePreloadTest : public DBTestBase { TEST_F(FilePreloadTest, PreloadCaching) { // create a DB with 3 files - // the day. ASSERT_OK(Put("key", "val")); ASSERT_OK(Flush()); ASSERT_OK(Put("key2", "val2")); @@ -1702,7 +1701,6 @@ TEST_F(FilePreloadTest, PreloadCaching) { TEST_F(FilePreloadTest, PreloadCorruption) { // create a DB with 3 files - // the day. ASSERT_OK(Put("key", "val")); ASSERT_OK(Flush()); ASSERT_OK(Put("key2", "val2")); @@ -1718,7 +1716,7 @@ TEST_F(FilePreloadTest, PreloadCorruption) { ASSERT_EQ(table_cache->GetUsage(), 3); Options new_options = GetOptions(kDefault); - ASSERT_TRUE(TryReopen(new_options).ok()); + ASSERT_OK(TryReopen(new_options)); db_impl = dbfull(); table_cache = db_impl->TEST_table_cache(); @@ -1734,13 +1732,13 @@ TEST_F(FilePreloadTest, PreloadCorruption) { std::string fail_file = meta.levels[0].files[0].db_path + meta.levels[0].files[0].name; std::string garbage(meta.levels[0].files[0].size, '@'); - ASSERT_TRUE(WriteStringToFile(db_->GetEnv(), garbage, fail_file, true).ok()); + ASSERT_OK(WriteStringToFile(db_->GetEnv(), garbage, fail_file, true)); - ASSERT_FALSE(TryReopen(new_options).ok()) + ASSERT_NOK(TryReopen(new_options)) << "reopen should fail with corrupted .sst"; new_options = GetOptions(kPreloadDisabled); - ASSERT_TRUE(TryReopen(new_options).ok()) + ASSERT_OK(TryReopen(new_options)) << "reopen should fail with preload disabled"; } #endif From aa8785b12159ad1ddb776fdb395a4a085f17254a Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 11 Jan 2022 17:09:49 -0500 Subject: [PATCH 32/39] convert kFilePreload to more generic kEnum --- include/rocksdb/utilities/options_type.h | 1 - options/cf_options.cc | 6 +++--- options/options_helper.cc | 16 ---------------- options/options_helper.h | 1 - 4 files changed, 3 insertions(+), 21 deletions(-) diff --git a/include/rocksdb/utilities/options_type.h b/include/rocksdb/utilities/options_type.h index 6bf0cdc0076..9ef02904c6a 100644 --- a/include/rocksdb/utilities/options_type.h +++ b/include/rocksdb/utilities/options_type.h @@ -35,7 +35,6 @@ enum class OptionType { kCompactionPri, kCompressionType, kCompactionStopStyle, - kFilePreload, kFilterPolicy, kChecksumType, kEncodingType, diff --git a/options/cf_options.cc b/options/cf_options.cc index 62e92ebd853..a9c9d423141 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -506,9 +506,9 @@ static std::unordered_map {0, OptionType::kBoolean, OptionVerificationType::kDeprecated, OptionTypeFlags::kNone}}, {"file_preload", - {offset_of(&ImmutableCFOptions::file_preload), - OptionType::kFilePreload, OptionVerificationType::kNormal, - OptionTypeFlags::kNone}}, + OptionTypeInfo::Enum( + offset_of(&ImmutableCFOptions::file_preload), + &file_preload_string_map)}, {"inplace_update_support", {offset_of(&ImmutableCFOptions::inplace_update_support), OptionType::kBoolean, OptionVerificationType::kNormal, diff --git a/options/options_helper.cc b/options/options_helper.cc index 3c1c849506c..507931c30b9 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -322,11 +322,6 @@ std::map OptionsHelper::compaction_pri_to_string = { {kOldestSmallestSeqFirst, "kOldestSmallestSeqFirst"}, {kMinOverlappingRatio, "kMinOverlappingRatio"}}; -std::map OptionsHelper::file_preload_to_string = { - {kFilePreloadWithPinning, "kFilePreloadWithPinning"}, - {kFilePreloadWithoutPinning, "kFilePreloadWithoutPinning"}, - {kFilePreloadDisabled, "kFilePreloadDisabled"}}; - std::map OptionsHelper::compaction_stop_style_to_string = { {kCompactionStopStyleSimilarSize, "kCompactionStopStyleSimilarSize"}, @@ -434,10 +429,6 @@ static bool ParseOptionHelper(void* opt_address, const OptionType& opt_type, return ParseEnum( compression_type_string_map, value, static_cast(opt_address)); - case OptionType::kFilePreload: - return ParseEnum( - file_preload_string_map, value, - reinterpret_cast(opt_address)); case OptionType::kChecksumType: return ParseEnum(checksum_type_string_map, value, static_cast(opt_address)); @@ -516,11 +507,6 @@ bool SerializeSingleOptionHelper(const void* opt_address, return SerializeEnum( compression_type_string_map, *(static_cast(opt_address)), value); - case OptionType::kFilePreload: - return SerializeEnum( - file_preload_string_map, - *(reinterpret_cast(opt_address)), value); - break; case OptionType::kFilterPolicy: { const auto* ptr = static_cast*>(opt_address); @@ -1200,8 +1186,6 @@ static bool AreOptionsEqual(OptionType type, const void* this_offset, return IsOptionEqual(this_offset, that_offset); case OptionType::kCompressionType: return IsOptionEqual(this_offset, that_offset); - case OptionType::kFilePreload: - return IsOptionEqual(this_offset, that_offset); case OptionType::kChecksumType: return IsOptionEqual(this_offset, that_offset); case OptionType::kEncodingType: diff --git a/options/options_helper.h b/options/options_helper.h index 1c2142eae55..2e382bf90c7 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -89,7 +89,6 @@ struct OptionsHelper { static std::unordered_map compaction_pri_string_map; #endif // !ROCKSDB_LITE - static std::map file_preload_to_string; static std::unordered_map file_preload_string_map; }; From 4e4a700ba5f79d0e3f55347389a398e3ec5dbbab Mon Sep 17 00:00:00 2001 From: matthewvon Date: Tue, 11 Jan 2022 17:27:10 -0500 Subject: [PATCH 33/39] make check bucks target rule happy ... white space changes --- options/cf_options.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/options/cf_options.cc b/options/cf_options.cc index a9c9d423141..9b0ea6627bb 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -505,10 +505,9 @@ static std::unordered_map {"compaction_measure_io_stats", {0, OptionType::kBoolean, OptionVerificationType::kDeprecated, OptionTypeFlags::kNone}}, - {"file_preload", - OptionTypeInfo::Enum( - offset_of(&ImmutableCFOptions::file_preload), - &file_preload_string_map)}, + {"file_preload", OptionTypeInfo::Enum( + offset_of(&ImmutableCFOptions::file_preload), + &file_preload_string_map)}, {"inplace_update_support", {offset_of(&ImmutableCFOptions::inplace_update_support), OptionType::kBoolean, OptionVerificationType::kNormal, @@ -903,9 +902,9 @@ uint64_t MultiplyCheckOverflow(uint64_t op1, double op2) { // when level_compaction_dynamic_level_bytes is true and leveled compaction // is used, the base level is not always L1, so precomupted max_file_size can // no longer be used. Recompute file_size_for_level from base level. -uint64_t MaxFileSizeForLevel(const MutableCFOptions& cf_options, - int level, CompactionStyle compaction_style, int base_level, - bool level_compaction_dynamic_level_bytes) { +uint64_t MaxFileSizeForLevel(const MutableCFOptions& cf_options, int level, + CompactionStyle compaction_style, int base_level, + bool level_compaction_dynamic_level_bytes) { if (!level_compaction_dynamic_level_bytes || level < base_level || compaction_style != kCompactionStyleLevel) { assert(level >= 0); From 15ab78ef8612e906f2c5880f9bc85eb24cff4c01 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Wed, 12 Jan 2022 12:20:04 -0500 Subject: [PATCH 34/39] per PR comments, rearrange code so that return Status usage more visible. --- db/version_builder.cc | 51 +++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index e54dd909062..ca3746e5589 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -1186,30 +1186,10 @@ class VersionBuilder::Rep { } } - Status ret; - if (max_load > 0) { - // - std::vector> files_meta; - std::vector statuses; - for (int level = 0; level < num_levels_; level++) { - for (auto& file_meta_pair : levels_[level].added_files) { - auto* file_meta = file_meta_pair.second; - // If the file has been opened before, just skip it. - if (!file_meta->table_reader_handle) { - files_meta.emplace_back(file_meta, level); - statuses.emplace_back(Status::OK()); - } - if (files_meta.size() >= max_load) { - break; - } - } - if (files_meta.size() >= max_load) { - break; - } - } - - std::atomic next_file_meta_idx(0); - std::function load_handlers_func([&]() { + // function called by multiple threads via loop + // that follows when preloading active + std::atomic next_file_meta_idx(0); + std::function load_handlers_func([&]() { while (true) { size_t file_idx = next_file_meta_idx.fetch_add(1); if (file_idx >= files_meta.size()) { @@ -1246,6 +1226,29 @@ class VersionBuilder::Rep { } }); + Status ret; + // Threaded preloading + if (max_load > 0) { + // + std::vector> files_meta; + std::vector statuses; + for (int level = 0; level < num_levels_; level++) { + for (auto& file_meta_pair : levels_[level].added_files) { + auto* file_meta = file_meta_pair.second; + // If the file has been opened before, just skip it. + if (!file_meta->table_reader_handle) { + files_meta.emplace_back(file_meta, level); + statuses.emplace_back(Status::OK()); + } + if (files_meta.size() >= max_load) { + break; + } + } + if (files_meta.size() >= max_load) { + break; + } + } + std::vector threads; for (int i = 1; i < max_threads; i++) { threads.emplace_back(load_handlers_func); From 943c251038933d17844dc9b77af019902e078003 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Wed, 12 Jan 2022 12:31:07 -0500 Subject: [PATCH 35/39] fix white space and move variable definitions for proper compile --- db/version_builder.cc | 70 +++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/db/version_builder.cc b/db/version_builder.cc index ca3746e5589..516579396c3 100644 --- a/db/version_builder.cc +++ b/db/version_builder.cc @@ -1186,52 +1186,52 @@ class VersionBuilder::Rep { } } + std::atomic next_file_meta_idx(0); + std::vector> files_meta; + std::vector statuses; + // function called by multiple threads via loop // that follows when preloading active - std::atomic next_file_meta_idx(0); std::function load_handlers_func([&]() { - while (true) { - size_t file_idx = next_file_meta_idx.fetch_add(1); - if (file_idx >= files_meta.size()) { - break; - } + while (true) { + size_t file_idx = next_file_meta_idx.fetch_add(1); + if (file_idx >= files_meta.size()) { + break; + } - auto* file_meta = files_meta[file_idx].first; - int level = files_meta[file_idx].second; - statuses[file_idx] = table_cache_->FindTable( - ReadOptions(), file_options_, - *(base_vstorage_->InternalComparator()), file_meta->fd, - &file_meta->table_reader_handle, prefix_extractor, - false /*no_io */, true /* record_read_stats */, - internal_stats->GetFileReadHist(level), false, level, - prefetch_index_and_filter_in_cache, max_file_size_for_l0_meta_pin, - file_meta->temperature); - - // The code is attempting two things: - // 1. preload / warm the table cache with new file objects - // 2. create higher performance via a cache lookup avoidance - // The issue is that number 2 creates permanent objects in the - // table cache which over time are no longer useful. The - // kFilePreloadWithoutPinning option keeps #1 and disables #2. - if (file_meta->table_reader_handle != nullptr) { - if (ioptions_->file_preload == kFilePreloadWithPinning) { - file_meta->fd.table_reader = - table_cache_->GetTableReaderFromHandle( - file_meta->table_reader_handle); - } else { // kFilePreloadWithoutPinning - table_cache_->ReleaseHandle(file_meta->table_reader_handle); - file_meta->table_reader_handle = nullptr; - } + auto* file_meta = files_meta[file_idx].first; + int level = files_meta[file_idx].second; + statuses[file_idx] = table_cache_->FindTable( + ReadOptions(), file_options_, + *(base_vstorage_->InternalComparator()), file_meta->fd, + &file_meta->table_reader_handle, prefix_extractor, false /*no_io */, + true /* record_read_stats */, + internal_stats->GetFileReadHist(level), false, level, + prefetch_index_and_filter_in_cache, max_file_size_for_l0_meta_pin, + file_meta->temperature); + + // The code is attempting two things: + // 1. preload / warm the table cache with new file objects + // 2. create higher performance via a cache lookup avoidance + // The issue is that number 2 creates permanent objects in the + // table cache which over time are no longer useful. The + // kFilePreloadWithoutPinning option keeps #1 and disables #2. + if (file_meta->table_reader_handle != nullptr) { + if (ioptions_->file_preload == kFilePreloadWithPinning) { + file_meta->fd.table_reader = table_cache_->GetTableReaderFromHandle( + file_meta->table_reader_handle); + } else { // kFilePreloadWithoutPinning + table_cache_->ReleaseHandle(file_meta->table_reader_handle); + file_meta->table_reader_handle = nullptr; } } - }); + } + }); Status ret; // Threaded preloading if (max_load > 0) { // - std::vector> files_meta; - std::vector statuses; for (int level = 0; level < num_levels_; level++) { for (auto& file_meta_pair : levels_[level].added_files) { auto* file_meta = file_meta_pair.second; From e9e61cbf35cad7dcf47b2968f8804b299f201b6e Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 11 Feb 2022 13:29:28 -0500 Subject: [PATCH 36/39] bad merge --- options/options_helper.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/options/options_helper.cc b/options/options_helper.cc index 4bbe3278547..2875958db6f 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -163,7 +163,6 @@ DBOptions BuildDBOptions(const ImmutableDBOptions& immutable_db_options, options.avoid_flush_during_shutdown = mutable_db_options.avoid_flush_during_shutdown; options.allow_ingest_behind = immutable_db_options.allow_ingest_behind; - options.preserve_deletes = immutable_db_options.preserve_deletes; options.two_write_queues = immutable_db_options.two_write_queues; options.manual_wal_flush = immutable_db_options.manual_wal_flush; options.wal_compression = immutable_db_options.wal_compression; From 6b183d27e06a43d1b65c14d9be293ead9c18dcba Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 11 Feb 2022 13:30:15 -0500 Subject: [PATCH 37/39] fix a spelling error --- tools/ldb_cmd.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 758b8ab33e5..4b205a43ad0 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -1863,7 +1863,7 @@ void DBDumperCommand::DoDumpCommand() { bucket_size <= 0) { bucket_size = time_range; // Will have just 1 bucket by default } - // cretaing variables for row count of each type + // creating variables for row count of each type std::string rtype1, rtype2, row, val; rtype2 = ""; uint64_t c = 0; From 53674ed0a9fe6df814b35818a077e0b334464665 Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 11 Feb 2022 14:18:46 -0500 Subject: [PATCH 38/39] per PR comments, move file_preload_string_map into cf_options.cc --- options/cf_options.cc | 6 ++++++ options/options_helper.cc | 8 +------- options/options_helper.h | 2 -- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/options/cf_options.cc b/options/cf_options.cc index ee0062e4110..0f8251024f0 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -140,6 +140,12 @@ static Status ParseCompressionOptions(const std::string& value, const std::string kOptNameBMCompOpts = "bottommost_compression_opts"; const std::string kOptNameCompOpts = "compression_opts"; +static std::unordered_map + file_preload_string_map = { + {"kFilePreloadWithPinning", FilePreload::kFilePreloadWithPinning}, + {"kFilePreloadWithoutPinning", FilePreload::kFilePreloadWithoutPinning}, + {"kFilePreloadDisabled", FilePreload::kFilePreloadDisabled}}; + // OptionTypeInfo map for CompressionOptions static std::unordered_map compression_options_type_info = { diff --git a/options/options_helper.cc b/options/options_helper.cc index 2875958db6f..31d75d95ca1 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -821,13 +821,7 @@ std::unordered_map {"kCompactionStopStyleSimilarSize", kCompactionStopStyleSimilarSize}, {"kCompactionStopStyleTotalSize", kCompactionStopStyleTotalSize}}; -std::unordered_map - OptionsHelper::file_preload_string_map = { - {"kFilePreloadWithPinning", FilePreload::kFilePreloadWithPinning}, - {"kFilePreloadWithoutPinning", FilePreload::kFilePreloadWithoutPinning}, - {"kFilePreloadDisabled", FilePreload::kFilePreloadDisabled}}; - - std::unordered_map +std::unordered_map OptionsHelper::temperature_string_map = { {"kUnknown", Temperature::kUnknown}, {"kHot", Temperature::kHot}, diff --git a/options/options_helper.h b/options/options_helper.h index c171c6ecc4c..b96ff160c4c 100644 --- a/options/options_helper.h +++ b/options/options_helper.h @@ -90,7 +90,6 @@ struct OptionsHelper { compaction_pri_string_map; static std::unordered_map temperature_string_map; #endif // !ROCKSDB_LITE - static std::unordered_map file_preload_string_map; }; // Some aliasing @@ -112,6 +111,5 @@ static auto& compaction_pri_string_map = OptionsHelper::compaction_pri_string_map; static auto& temperature_string_map = OptionsHelper::temperature_string_map; #endif // !ROCKSDB_LITE -static auto& file_preload_string_map = OptionsHelper::file_preload_string_map; } // namespace ROCKSDB_NAMESPACE From a02e47e19b949e4fa0a2706035537ff896365c8e Mon Sep 17 00:00:00 2001 From: matthewvon Date: Fri, 11 Feb 2022 16:23:17 -0500 Subject: [PATCH 39/39] formatting update for CI happiness --- options/cf_options.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/options/cf_options.cc b/options/cf_options.cc index 0f8251024f0..c6eac21cd28 100644 --- a/options/cf_options.cc +++ b/options/cf_options.cc @@ -140,11 +140,10 @@ static Status ParseCompressionOptions(const std::string& value, const std::string kOptNameBMCompOpts = "bottommost_compression_opts"; const std::string kOptNameCompOpts = "compression_opts"; -static std::unordered_map - file_preload_string_map = { - {"kFilePreloadWithPinning", FilePreload::kFilePreloadWithPinning}, - {"kFilePreloadWithoutPinning", FilePreload::kFilePreloadWithoutPinning}, - {"kFilePreloadDisabled", FilePreload::kFilePreloadDisabled}}; +static std::unordered_map file_preload_string_map = { + {"kFilePreloadWithPinning", FilePreload::kFilePreloadWithPinning}, + {"kFilePreloadWithoutPinning", FilePreload::kFilePreloadWithoutPinning}, + {"kFilePreloadDisabled", FilePreload::kFilePreloadDisabled}}; // OptionTypeInfo map for CompressionOptions static std::unordered_map