From 270179bb125e82ce1c6cca1e6f5edad03196548b Mon Sep 17 00:00:00 2001 From: Jay Zhuang Date: Wed, 4 May 2022 08:49:46 -0700 Subject: [PATCH] Default `try_load_options` to true when DB is specified (#9937) Summary: If the DB path is specified, the user would expect ldb loads the options from the path, but it's not: ``` $ ldb list_live_files_metadata --db=`pwd` ``` Default `try_load_options` to true in that case. The user can still disable that by: ``` $ ldb list_live_files_metadata --db=`pwd` --try_load_options=false ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/9937 Test Plan: `ldb list_live_files_metadata --db=`pwd`` is able to work for a db generated with different options.num_levels. Reviewed By: ajkr Differential Revision: D36106708 Pulled By: jay-zhuang fbshipit-source-id: 2732fdc027a4d172436b2c9b6a9787b56b10c710 --- HISTORY.md | 1 + include/rocksdb/utilities/ldb_cmd.h | 3 +++ tools/ldb_cmd.cc | 20 +++++++++++++++++++- tools/ldb_tool.cc | 5 ++++- 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index baf61150a..6c640422d 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -21,6 +21,7 @@ ### Behavior changes * Enforce the existing contract of SingleDelete so that SingleDelete cannot be mixed with Delete because it leads to undefined behavior. Fix a number of unit tests that violate the contract but happen to pass. +* ldb `--try_load_options` default to true if `--db` is specified and not creating a new DB, the user can still explicitly disable that by `--try_load_options=false` (or explicitly enable that by `--try_load_options`). ## 7.2.0 (04/15/2022) ### Bug Fixes diff --git a/include/rocksdb/utilities/ldb_cmd.h b/include/rocksdb/utilities/ldb_cmd.h index 71fedc5d4..e75d21857 100644 --- a/include/rocksdb/utilities/ldb_cmd.h +++ b/include/rocksdb/utilities/ldb_cmd.h @@ -288,6 +288,9 @@ class LDBCommand { bool IsValueHex(const std::map& options, const std::vector& flags); + bool IsTryLoadOptions(const std::map& options, + const std::vector& flags); + /** * Converts val to a boolean. * val must be either true or false (case insensitive). diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 2228ea47e..91a0b2775 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -408,7 +408,7 @@ LDBCommand::LDBCommand(const std::map& options, is_value_hex_ = IsValueHex(options, flags); is_db_ttl_ = IsFlagPresent(flags, ARG_TTL); timestamp_ = IsFlagPresent(flags, ARG_TIMESTAMP); - try_load_options_ = IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS); + try_load_options_ = IsTryLoadOptions(options, flags); force_consistency_checks_ = !IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS); enable_blob_files_ = IsFlagPresent(flags, ARG_ENABLE_BLOB_FILES); @@ -1064,6 +1064,24 @@ bool LDBCommand::IsValueHex(const std::map& options, ParseBooleanOption(options, ARG_VALUE_HEX, false)); } +bool LDBCommand::IsTryLoadOptions( + const std::map& options, + const std::vector& flags) { + if (IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS)) { + return true; + } + // if `DB` is specified and not explicitly to create a new db, default + // `try_load_options` to true. The user could still disable that by set + // `try_load_options=false`. + // Note: Opening as TTL DB doesn't support `try_load_options`, so it's default + // to false. TODO: TTL_DB may need to fix that, otherwise it's unable to open + // DB which has incompatible setting with default options. + bool default_val = (options.find(ARG_DB) != options.end()) && + !IsFlagPresent(flags, ARG_CREATE_IF_MISSING) && + !IsFlagPresent(flags, ARG_TTL); + return ParseBooleanOption(options, ARG_TRY_LOAD_OPTIONS, default_val); +} + bool LDBCommand::ParseBooleanOption( const std::map& options, const std::string& option, bool default_val) { diff --git a/tools/ldb_tool.cc b/tools/ldb_tool.cc index e3c684b66..402516419 100644 --- a/tools/ldb_tool.cc +++ b/tools/ldb_tool.cc @@ -50,7 +50,10 @@ void LDBCommandRunner::PrintHelp(const LDBOptions& ldb_options, " with 'put','get','scan','dump','query','batchput'" " : DB supports ttl and value is internally timestamp-suffixed\n"); ret.append(" --" + LDBCommand::ARG_TRY_LOAD_OPTIONS + - " : Try to load option file from DB.\n"); + " : Try to load option file from DB. Default to true if " + + LDBCommand::ARG_DB + + " is specified and not creating a new DB and not open as TTL DB. " + "Can be set to false explicitly.\n"); ret.append(" --" + LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS + " : Set options.force_consistency_checks = false.\n"); ret.append(" --" + LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS +