From 8e7b068ecc00d0e21e7b8539c81db87d6b199818 Mon Sep 17 00:00:00 2001 From: anand76 Date: Mon, 11 Jan 2021 20:54:58 -0800 Subject: [PATCH] Make ldb load column family options from OPTIONS file (#7847) Summary: When the --try_load_options is used in conjunction with the --column_family option, ldb incorrectly sets the ColumnFamilyOptions for that column family to defaults. This PR fixes that by retaining from the OPTIONS file and applying command line overrides. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7847 Test Plan: Add a unit test in ldb_cmd_test Reviewed By: ajkr Differential Revision: D25874720 Pulled By: anand1976 fbshipit-source-id: 04bcf23b55e5a30b5b6a59b0e5cb4faef3da7429 --- HISTORY.md | 1 + include/rocksdb/utilities/ldb_cmd.h | 2 + tools/ldb_cmd.cc | 97 +++++++++++++++-------------- tools/ldb_cmd_impl.h | 4 +- tools/ldb_cmd_test.cc | 47 ++++++++++++++ 5 files changed, 103 insertions(+), 48 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 923f2e449..96909e732 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -23,6 +23,7 @@ * Fixed prefix extractor with timestamp issues. * Fixed a bug in atomic flush: in two-phase commit mode, the minimum WAL log number to keep is incorrect. * Fixed a bug related to checkpoint in PR7789: if there are multiple column families, and the checkpoint is not opened as read only, then in rare cases, data loss may happen in the checkpoint. Since backup engine relies on checkpoint, it may also be affected. +* When ldb --try_load_options is used with the --column_family option, the ColumnFamilyOptions for the specified column family was not loaded from the OPTIONS file. Fix it so its loaded from OPTIONS and then overridden with command line overrides. ### New Features * User defined timestamp feature supports `CompactRange` and `GetApproximateSizes`. diff --git a/include/rocksdb/utilities/ldb_cmd.h b/include/rocksdb/utilities/ldb_cmd.h index c7f227fc0..af0556a32 100644 --- a/include/rocksdb/utilities/ldb_cmd.h +++ b/include/rocksdb/utilities/ldb_cmd.h @@ -88,6 +88,8 @@ class LDBCommand { virtual void OverrideBaseOptions(); + virtual void OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts); + virtual void SetDBOptions(Options options) { options_ = options; } virtual void SetColumnFamilies( diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index cf5819042..e1d0f6084 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -553,6 +553,26 @@ bool LDBCommand::ParseStringOption( void LDBCommand::OverrideBaseOptions() { options_.create_if_missing = false; + int db_write_buffer_size; + if (ParseIntOption(option_map_, ARG_DB_WRITE_BUFFER_SIZE, + db_write_buffer_size, exec_state_)) { + if (db_write_buffer_size >= 0) { + options_.db_write_buffer_size = db_write_buffer_size; + } else { + exec_state_ = LDBCommandExecuteResult::Failed(ARG_DB_WRITE_BUFFER_SIZE + + " must be >= 0."); + } + } + + if (options_.db_paths.size() == 0) { + options_.db_paths.emplace_back(db_path_, + std::numeric_limits::max()); + } + + OverrideBaseCFOptions(static_cast(&options_)); +} + +void LDBCommand::OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) { BlockBasedTableOptions table_options; bool use_table_options = false; int bits; @@ -577,35 +597,35 @@ void LDBCommand::OverrideBaseOptions() { } } - options_.force_consistency_checks = force_consistency_checks_; + cf_opts->force_consistency_checks = force_consistency_checks_; if (use_table_options) { - options_.table_factory.reset(NewBlockBasedTableFactory(table_options)); + cf_opts->table_factory.reset(NewBlockBasedTableFactory(table_options)); } auto itr = option_map_.find(ARG_AUTO_COMPACTION); if (itr != option_map_.end()) { - options_.disable_auto_compactions = !StringToBool(itr->second); + cf_opts->disable_auto_compactions = !StringToBool(itr->second); } itr = option_map_.find(ARG_COMPRESSION_TYPE); if (itr != option_map_.end()) { std::string comp = itr->second; if (comp == "no") { - options_.compression = kNoCompression; + cf_opts->compression = kNoCompression; } else if (comp == "snappy") { - options_.compression = kSnappyCompression; + cf_opts->compression = kSnappyCompression; } else if (comp == "zlib") { - options_.compression = kZlibCompression; + cf_opts->compression = kZlibCompression; } else if (comp == "bzip2") { - options_.compression = kBZip2Compression; + cf_opts->compression = kBZip2Compression; } else if (comp == "lz4") { - options_.compression = kLZ4Compression; + cf_opts->compression = kLZ4Compression; } else if (comp == "lz4hc") { - options_.compression = kLZ4HCCompression; + cf_opts->compression = kLZ4HCCompression; } else if (comp == "xpress") { - options_.compression = kXpressCompression; + cf_opts->compression = kXpressCompression; } else if (comp == "zstd") { - options_.compression = kZSTD; + cf_opts->compression = kZSTD; } else { // Unknown compression. exec_state_ = @@ -617,29 +637,18 @@ void LDBCommand::OverrideBaseOptions() { if (ParseIntOption(option_map_, ARG_COMPRESSION_MAX_DICT_BYTES, compression_max_dict_bytes, exec_state_)) { if (compression_max_dict_bytes >= 0) { - options_.compression_opts.max_dict_bytes = compression_max_dict_bytes; + cf_opts->compression_opts.max_dict_bytes = compression_max_dict_bytes; } else { exec_state_ = LDBCommandExecuteResult::Failed( ARG_COMPRESSION_MAX_DICT_BYTES + " must be >= 0."); } } - int db_write_buffer_size; - if (ParseIntOption(option_map_, ARG_DB_WRITE_BUFFER_SIZE, - db_write_buffer_size, exec_state_)) { - if (db_write_buffer_size >= 0) { - options_.db_write_buffer_size = db_write_buffer_size; - } else { - exec_state_ = LDBCommandExecuteResult::Failed(ARG_DB_WRITE_BUFFER_SIZE + - " must be >= 0."); - } - } - int write_buffer_size; if (ParseIntOption(option_map_, ARG_WRITE_BUFFER_SIZE, write_buffer_size, exec_state_)) { if (write_buffer_size > 0) { - options_.write_buffer_size = write_buffer_size; + cf_opts->write_buffer_size = write_buffer_size; } else { exec_state_ = LDBCommandExecuteResult::Failed(ARG_WRITE_BUFFER_SIZE + " must be > 0."); @@ -649,23 +658,18 @@ void LDBCommand::OverrideBaseOptions() { int file_size; if (ParseIntOption(option_map_, ARG_FILE_SIZE, file_size, exec_state_)) { if (file_size > 0) { - options_.target_file_size_base = file_size; + cf_opts->target_file_size_base = file_size; } else { exec_state_ = LDBCommandExecuteResult::Failed(ARG_FILE_SIZE + " must be > 0."); } } - if (options_.db_paths.size() == 0) { - options_.db_paths.emplace_back(db_path_, - std::numeric_limits::max()); - } - int fix_prefix_len; if (ParseIntOption(option_map_, ARG_FIX_PREFIX_LEN, fix_prefix_len, exec_state_)) { if (fix_prefix_len > 0) { - options_.prefix_extractor.reset( + cf_opts->prefix_extractor.reset( NewFixedPrefixTransform(static_cast(fix_prefix_len))); } else { exec_state_ = @@ -740,7 +744,7 @@ void LDBCommand::PrepareOptions() { "Non-existing column family " + column_family_name_); return; } - column_families_iter->options = options_; + OverrideBaseCFOptions(&column_families_iter->options); } } @@ -1947,14 +1951,15 @@ void ReduceDBLevelsCommand::Help(std::string& ret) { ret.append("\n"); } -void ReduceDBLevelsCommand::OverrideBaseOptions() { - LDBCommand::OverrideBaseOptions(); - options_.num_levels = old_levels_; - options_.max_bytes_for_level_multiplier_additional.resize(options_.num_levels, +void ReduceDBLevelsCommand::OverrideBaseCFOptions( + ColumnFamilyOptions* cf_opts) { + LDBCommand::OverrideBaseCFOptions(cf_opts); + cf_opts->num_levels = old_levels_; + cf_opts->max_bytes_for_level_multiplier_additional.resize(cf_opts->num_levels, 1); // Disable size compaction - options_.max_bytes_for_level_base = 1ULL << 50; - options_.max_bytes_for_level_multiplier = 1; + cf_opts->max_bytes_for_level_base = 1ULL << 50; + cf_opts->max_bytes_for_level_multiplier = 1; } Status ReduceDBLevelsCommand::GetOldNumOfLevels(Options& opt, @@ -2100,18 +2105,18 @@ void ChangeCompactionStyleCommand::Help(std::string& ret) { ret.append("\n"); } -void ChangeCompactionStyleCommand::OverrideBaseOptions() { - LDBCommand::OverrideBaseOptions(); - +void ChangeCompactionStyleCommand::OverrideBaseCFOptions( + ColumnFamilyOptions* cf_opts) { + LDBCommand::OverrideBaseCFOptions(cf_opts); if (old_compaction_style_ == kCompactionStyleLevel && new_compaction_style_ == kCompactionStyleUniversal) { // In order to convert from level compaction to universal compaction, we // need to compact all data into a single file and move it to level 0. - options_.disable_auto_compactions = true; - options_.target_file_size_base = INT_MAX; - options_.target_file_size_multiplier = 1; - options_.max_bytes_for_level_base = INT_MAX; - options_.max_bytes_for_level_multiplier = 1; + cf_opts->disable_auto_compactions = true; + cf_opts->target_file_size_base = INT_MAX; + cf_opts->target_file_size_multiplier = 1; + cf_opts->max_bytes_for_level_base = INT_MAX; + cf_opts->max_bytes_for_level_multiplier = 1; } } diff --git a/tools/ldb_cmd_impl.h b/tools/ldb_cmd_impl.h index c0bfc19aa..d20f5b98e 100644 --- a/tools/ldb_cmd_impl.h +++ b/tools/ldb_cmd_impl.h @@ -262,7 +262,7 @@ class ReduceDBLevelsCommand : public LDBCommand { const std::map& options, const std::vector& flags); - virtual void OverrideBaseOptions() override; + virtual void OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) override; virtual void DoCommand() override; @@ -294,7 +294,7 @@ class ChangeCompactionStyleCommand : public LDBCommand { const std::map& options, const std::vector& flags); - virtual void OverrideBaseOptions() override; + virtual void OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) override; virtual void DoCommand() override; diff --git a/tools/ldb_cmd_test.cc b/tools/ldb_cmd_test.cc index 504a5d72c..159d752d8 100644 --- a/tools/ldb_cmd_test.cc +++ b/tools/ldb_cmd_test.cc @@ -13,6 +13,7 @@ #include "file/filename.h" #include "port/stack_trace.h" #include "rocksdb/file_checksum.h" +#include "rocksdb/utilities/options_util.h" #include "test_util/sync_point.h" #include "test_util/testharness.h" #include "test_util/testutil.h" @@ -672,6 +673,52 @@ TEST_F(LdbCmdTest, TestBadDbPath) { ASSERT_EQ(1, LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr)); } + +TEST_F(LdbCmdTest, LoadCFOptionsAndOverride) { + // Env* base_env = TryLoadCustomOrDefaultEnv(); + // std::unique_ptr env(NewMemEnv(base_env)); + std::unique_ptr env(new EnvWrapper(Env::Default())); + Options opts; + opts.env = env.get(); + opts.create_if_missing = true; + + DB* db = nullptr; + std::string dbname = test::PerThreadDBPath(env.get(), "ldb_cmd_test"); + DestroyDB(dbname, opts); + ASSERT_OK(DB::Open(opts, dbname, &db)); + + ColumnFamilyHandle* cf_handle; + ColumnFamilyOptions cf_opts; + cf_opts.num_levels = 20; + ASSERT_OK(db->CreateColumnFamily(cf_opts, "cf1", &cf_handle)); + + delete cf_handle; + delete db; + + char arg1[] = "./ldb"; + char arg2[1024]; + snprintf(arg2, sizeof(arg2), "--db=%s", dbname.c_str()); + char arg3[] = "put"; + char arg4[] = "key1"; + char arg5[] = "value1"; + char arg6[] = "--try_load_options"; + char arg7[] = "--column_family=cf1"; + char arg8[] = "--write_buffer_size=268435456"; + char* argv[] = {arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8}; + + ASSERT_EQ(0, + LDBCommandRunner::RunCommand(8, argv, opts, LDBOptions(), nullptr)); + + ConfigOptions config_opts; + Options options; + std::vector column_families; + config_opts.env = env.get(); + ASSERT_OK(LoadLatestOptions(config_opts, dbname, &options, &column_families)); + ASSERT_EQ(column_families.size(), 2); + ASSERT_EQ(options.num_levels, opts.num_levels); + ASSERT_EQ(column_families[1].options.num_levels, cf_opts.num_levels); + ASSERT_EQ(column_families[1].options.write_buffer_size, 268435456); +} } // namespace ROCKSDB_NAMESPACE #ifdef ROCKSDB_UNITTESTS_WITH_CUSTOM_OBJECTS_FROM_STATIC_LIBS