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
This commit is contained in:
Jay Zhuang 2022-05-04 08:49:46 -07:00 committed by Facebook GitHub Bot
parent 8b74cea7fe
commit 270179bb12
4 changed files with 27 additions and 2 deletions

View File

@ -21,6 +21,7 @@
### Behavior changes ### 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. * 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) ## 7.2.0 (04/15/2022)
### Bug Fixes ### Bug Fixes

View File

@ -288,6 +288,9 @@ class LDBCommand {
bool IsValueHex(const std::map<std::string, std::string>& options, bool IsValueHex(const std::map<std::string, std::string>& options,
const std::vector<std::string>& flags); const std::vector<std::string>& flags);
bool IsTryLoadOptions(const std::map<std::string, std::string>& options,
const std::vector<std::string>& flags);
/** /**
* Converts val to a boolean. * Converts val to a boolean.
* val must be either true or false (case insensitive). * val must be either true or false (case insensitive).

View File

@ -408,7 +408,7 @@ LDBCommand::LDBCommand(const std::map<std::string, std::string>& options,
is_value_hex_ = IsValueHex(options, flags); is_value_hex_ = IsValueHex(options, flags);
is_db_ttl_ = IsFlagPresent(flags, ARG_TTL); is_db_ttl_ = IsFlagPresent(flags, ARG_TTL);
timestamp_ = IsFlagPresent(flags, ARG_TIMESTAMP); timestamp_ = IsFlagPresent(flags, ARG_TIMESTAMP);
try_load_options_ = IsFlagPresent(flags, ARG_TRY_LOAD_OPTIONS); try_load_options_ = IsTryLoadOptions(options, flags);
force_consistency_checks_ = force_consistency_checks_ =
!IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS); !IsFlagPresent(flags, ARG_DISABLE_CONSISTENCY_CHECKS);
enable_blob_files_ = IsFlagPresent(flags, ARG_ENABLE_BLOB_FILES); enable_blob_files_ = IsFlagPresent(flags, ARG_ENABLE_BLOB_FILES);
@ -1064,6 +1064,24 @@ bool LDBCommand::IsValueHex(const std::map<std::string, std::string>& options,
ParseBooleanOption(options, ARG_VALUE_HEX, false)); ParseBooleanOption(options, ARG_VALUE_HEX, false));
} }
bool LDBCommand::IsTryLoadOptions(
const std::map<std::string, std::string>& options,
const std::vector<std::string>& 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( bool LDBCommand::ParseBooleanOption(
const std::map<std::string, std::string>& options, const std::map<std::string, std::string>& options,
const std::string& option, bool default_val) { const std::string& option, bool default_val) {

View File

@ -50,7 +50,10 @@ void LDBCommandRunner::PrintHelp(const LDBOptions& ldb_options,
" with 'put','get','scan','dump','query','batchput'" " with 'put','get','scan','dump','query','batchput'"
" : DB supports ttl and value is internally timestamp-suffixed\n"); " : DB supports ttl and value is internally timestamp-suffixed\n");
ret.append(" --" + LDBCommand::ARG_TRY_LOAD_OPTIONS + 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 + ret.append(" --" + LDBCommand::ARG_DISABLE_CONSISTENCY_CHECKS +
" : Set options.force_consistency_checks = false.\n"); " : Set options.force_consistency_checks = false.\n");
ret.append(" --" + LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS + ret.append(" --" + LDBCommand::ARG_IGNORE_UNKNOWN_OPTIONS +