dolysis
/
rocksdb
mirror of https://github.com/facebook/rocksdb.git
3
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve error message from `SanityCheckCFOptions()` for merge_operator (#11393) 

Summary:
This happens when the persisted merge operator not a RocksDB built-in one. This PR improves this error message to include the actual persisted merge operator name. when there is a merge_operator mismatch in `SanityCheckCFOptions()`, for example, going from merge operator "CustomMergeOp" to nullptr, an error message like the following is returned:

"failed the verification on ColumnFamilyOptions::merge_operator--- The specified one is nullptr while the **persisted one is nullptr**."

This happens when the persisted merge operator not a RocksDB built-in one. This PR improves this error message to include the actual persisted merge operator name.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/11393

Test Plan: add unit test to check error message when going from merge op -> nullptr and going from merge op1 to merge op 2.

Reviewed By: ajkr

Differential Revision: D45190131

Pulled By: cbi42

fbshipit-source-id: 67712c2fec29c654c15166d1be985e710e6081e5
This commit is contained in:
Changyu Bi 2023-04-21 16:54:02 -07:00 committed by Facebook GitHub Bot
parent 151242ce46
commit adc9001f20
2 changed files with 39 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
options/options_parser.cc
View File

@ -679,6 +679,15 @@ Status RocksDBOptionsParser::VerifyCFOptions(
Status s = base_config->GetOption(config_options, mismatch, &base_value);
if (s.ok()) {
s = file_config->GetOption(config_options, mismatch, &file_value);
// In file_opt, certain options like MergeOperator may be nullptr due to
// factor methods not available. So we use opt_map to get
// option value to use in the error message below.
if (s.ok() && file_value == kNullptrString && opt_map) {
auto const& opt_val_str = (opt_map->find(mismatch));
if (opt_val_str != opt_map->end()) {
file_value = opt_val_str->second;
}
}
}
int offset = snprintf(buffer, sizeof(buffer),
"[RocksDBOptionsParser]: "

30
options/options_test.cc
View File

@ -3927,6 +3927,36 @@ class OptionsSanityCheckTest : public OptionsParserTest,
const std::string kOptionsFileName = "OPTIONS";
};
TEST_P(OptionsSanityCheckTest, MergeOperatorErrorMessage) {
ColumnFamilyOptions opts;
Random rnd(301);
opts.merge_operator.reset(test::RandomMergeOperator(&rnd));
std::string merge_op_name = opts.merge_operator->Name();
ASSERT_OK(PersistCFOptions(opts));
// Test when going from merge operator -> nullptr
opts.merge_operator = nullptr;
Status s =
SanityCheckCFOptions(opts, ConfigOptions::kSanityLevelLooselyCompatible);
ASSERT_TRUE(s.IsInvalidArgument());
std::string err_msg = s.ToString();
std::string specified = "The specified one is " + kNullptrString;
std::string persisted = "the persisted one is " + merge_op_name;
ASSERT_TRUE(err_msg.find(specified) != std::string::npos);
ASSERT_TRUE(err_msg.find(persisted) != std::string::npos);
// Test when using a different merge operator
opts.merge_operator.reset(test::RandomMergeOperator(&rnd));
s = SanityCheckCFOptions(opts, ConfigOptions::kSanityLevelLooselyCompatible);
ASSERT_TRUE(s.IsInvalidArgument());
err_msg = s.ToString();
specified =
"The specified one is " + std::string(opts.merge_operator->Name());
persisted = "the persisted one is " + merge_op_name;
ASSERT_TRUE(err_msg.find(specified) != std::string::npos);
ASSERT_TRUE(err_msg.find(persisted) != std::string::npos);
}
TEST_P(OptionsSanityCheckTest, CFOptionsSanityCheck) {
ColumnFamilyOptions opts;
Random rnd(301);