diff --git a/tools/db_bench_tool.cc b/tools/db_bench_tool.cc index f356e08f4..586a7884a 100644 --- a/tools/db_bench_tool.cc +++ b/tools/db_bench_tool.cc @@ -3857,6 +3857,26 @@ class Benchmark { assert(db_.db == nullptr); options.env = FLAGS_env; + options.wal_dir = FLAGS_wal_dir; + options.dump_malloc_stats = FLAGS_dump_malloc_stats; + options.stats_dump_period_sec = + static_cast(FLAGS_stats_dump_period_sec); + options.stats_persist_period_sec = + static_cast(FLAGS_stats_persist_period_sec); + options.persist_stats_to_disk = FLAGS_persist_stats_to_disk; + options.stats_history_buffer_size = + static_cast(FLAGS_stats_history_buffer_size); + options.avoid_flush_during_recovery = FLAGS_avoid_flush_during_recovery; + + options.compression_opts.level = FLAGS_compression_level; + options.compression_opts.max_dict_bytes = FLAGS_compression_max_dict_bytes; + options.compression_opts.zstd_max_train_bytes = + FLAGS_compression_zstd_max_train_bytes; + options.compression_opts.parallel_threads = + FLAGS_compression_parallel_threads; + options.compression_opts.max_dict_buffer_bytes = + FLAGS_compression_max_dict_buffer_bytes; + options.max_open_files = FLAGS_open_files; if (FLAGS_cost_write_buffer_to_cache || FLAGS_db_write_buffer_size != 0) { options.write_buffer_manager.reset( @@ -4285,94 +4305,104 @@ class Benchmark { } void InitializeOptionsGeneral(Options* opts) { + // Be careful about what is set here to avoid accidentally overwriting + // settings already configured by OPTIONS file. Only configure settings that + // are needed for the benchmark to run, settings for shared objects that + // were not configured already, settings that require dynamically invoking + // APIs, and settings for the benchmark itself. Options& options = *opts; - options.create_missing_column_families = FLAGS_num_column_families > 1; - options.statistics = dbstats; - options.wal_dir = FLAGS_wal_dir; - options.create_if_missing = !FLAGS_use_existing_db; - options.dump_malloc_stats = FLAGS_dump_malloc_stats; - options.stats_dump_period_sec = - static_cast(FLAGS_stats_dump_period_sec); - options.stats_persist_period_sec = - static_cast(FLAGS_stats_persist_period_sec); - options.persist_stats_to_disk = FLAGS_persist_stats_to_disk; - options.stats_history_buffer_size = - static_cast(FLAGS_stats_history_buffer_size); - options.avoid_flush_during_recovery = FLAGS_avoid_flush_during_recovery; + // Always set these since they are harmless when not needed and prevent + // a guaranteed failure when they are needed. + options.create_missing_column_families = true; + options.create_if_missing = true; + + if (options.statistics == nullptr) { + options.statistics = dbstats; + } - options.compression_opts.level = FLAGS_compression_level; - options.compression_opts.max_dict_bytes = FLAGS_compression_max_dict_bytes; - options.compression_opts.zstd_max_train_bytes = - FLAGS_compression_zstd_max_train_bytes; - options.compression_opts.parallel_threads = - FLAGS_compression_parallel_threads; - options.compression_opts.max_dict_buffer_bytes = - FLAGS_compression_max_dict_buffer_bytes; - // If this is a block based table, set some related options auto table_options = options.table_factory->GetOptions(); if (table_options != nullptr) { - if (FLAGS_cache_size) { + if (FLAGS_cache_size > 0) { + // This violates this function's rules on when to set options. But we + // have to do it because the case of unconfigured block cache in OPTIONS + // file is indistinguishable (it is sanitized to 8MB by this point, not + // nullptr), and our regression tests assume this will be the shared + // block cache, even with OPTIONS file provided. table_options->block_cache = cache_; } - if (FLAGS_bloom_bits < 0) { - table_options->filter_policy = BlockBasedTableOptions().filter_policy; - } else if (FLAGS_bloom_bits == 0) { - table_options->filter_policy.reset(); - } else if (FLAGS_use_block_based_filter) { - // Use back-door way of enabling obsolete block-based Bloom - Status s = FilterPolicy::CreateFromString( - ConfigOptions(), - "rocksdb.internal.DeprecatedBlockBasedBloomFilter:" + - ROCKSDB_NAMESPACE::ToString(FLAGS_bloom_bits), - &table_options->filter_policy); - if (!s.ok()) { - fprintf(stderr, "failure creating obsolete block-based filter: %s\n", - s.ToString().c_str()); - exit(1); + if (table_options->filter_policy == nullptr) { + if (FLAGS_bloom_bits < 0) { + table_options->filter_policy = BlockBasedTableOptions().filter_policy; + } else if (FLAGS_bloom_bits == 0) { + table_options->filter_policy.reset(); + } else if (FLAGS_use_block_based_filter) { + // Use back-door way of enabling obsolete block-based Bloom + Status s = FilterPolicy::CreateFromString( + ConfigOptions(), + "rocksdb.internal.DeprecatedBlockBasedBloomFilter:" + + ROCKSDB_NAMESPACE::ToString(FLAGS_bloom_bits), + &table_options->filter_policy); + if (!s.ok()) { + fprintf(stderr, + "failure creating obsolete block-based filter: %s\n", + s.ToString().c_str()); + exit(1); + } + } else { + table_options->filter_policy.reset( + FLAGS_use_ribbon_filter ? NewRibbonFilterPolicy(FLAGS_bloom_bits) + : NewBloomFilterPolicy(FLAGS_bloom_bits)); } - } else { - table_options->filter_policy.reset( - FLAGS_use_ribbon_filter ? NewRibbonFilterPolicy(FLAGS_bloom_bits) - : NewBloomFilterPolicy(FLAGS_bloom_bits)); } } - if (FLAGS_row_cache_size) { - if (FLAGS_cache_numshardbits >= 1) { - options.row_cache = - NewLRUCache(FLAGS_row_cache_size, FLAGS_cache_numshardbits); - } else { - options.row_cache = NewLRUCache(FLAGS_row_cache_size); + + if (options.row_cache == nullptr) { + if (FLAGS_row_cache_size) { + if (FLAGS_cache_numshardbits >= 1) { + options.row_cache = + NewLRUCache(FLAGS_row_cache_size, FLAGS_cache_numshardbits); + } else { + options.row_cache = NewLRUCache(FLAGS_row_cache_size); + } } } + + if (options.env == Env::Default()) { + options.env = FLAGS_env; + } if (FLAGS_enable_io_prio) { - FLAGS_env->LowerThreadPoolIOPriority(Env::LOW); - FLAGS_env->LowerThreadPoolIOPriority(Env::HIGH); + options.env->LowerThreadPoolIOPriority(Env::LOW); + options.env->LowerThreadPoolIOPriority(Env::HIGH); } if (FLAGS_enable_cpu_prio) { - FLAGS_env->LowerThreadPoolCPUPriority(Env::LOW); - FLAGS_env->LowerThreadPoolCPUPriority(Env::HIGH); + options.env->LowerThreadPoolCPUPriority(Env::LOW); + options.env->LowerThreadPoolCPUPriority(Env::HIGH); } - options.env = FLAGS_env; + if (FLAGS_sine_write_rate) { FLAGS_benchmark_write_rate_limit = static_cast(SineRate(0)); } - if (FLAGS_rate_limiter_bytes_per_sec > 0) { - options.rate_limiter.reset(NewGenericRateLimiter( - FLAGS_rate_limiter_bytes_per_sec, FLAGS_rate_limiter_refill_period_us, - 10 /* fairness */, - FLAGS_rate_limit_bg_reads ? RateLimiter::Mode::kReadsOnly - : RateLimiter::Mode::kWritesOnly, - FLAGS_rate_limiter_auto_tuned)); + if (options.rate_limiter == nullptr) { + if (FLAGS_rate_limiter_bytes_per_sec > 0) { + options.rate_limiter.reset(NewGenericRateLimiter( + FLAGS_rate_limiter_bytes_per_sec, + FLAGS_rate_limiter_refill_period_us, 10 /* fairness */, + FLAGS_rate_limit_bg_reads ? RateLimiter::Mode::kReadsOnly + : RateLimiter::Mode::kWritesOnly, + FLAGS_rate_limiter_auto_tuned)); + } } options.listeners.emplace_back(listener_); - if (FLAGS_file_checksum) { - options.file_checksum_gen_factory.reset( - new FileChecksumGenCrc32cFactory()); + if (options.file_checksum_gen_factory == nullptr) { + if (FLAGS_file_checksum) { + options.file_checksum_gen_factory.reset( + new FileChecksumGenCrc32cFactory()); + } } if (FLAGS_num_multi_db <= 1) { @@ -4391,9 +4421,11 @@ class Benchmark { } // KeepFilter is a noop filter, this can be used to test compaction filter - if (FLAGS_use_keep_filter) { - options.compaction_filter = new KeepFilter(); - fprintf(stdout, "A noop compaction filter is used\n"); + if (options.compaction_filter == nullptr) { + if (FLAGS_use_keep_filter) { + options.compaction_filter = new KeepFilter(); + fprintf(stdout, "A noop compaction filter is used\n"); + } } if (FLAGS_use_existing_keys) {