mirror of https://github.com/facebook/rocksdb.git
Honor ConfigOptions.ignore_unknown_options in ParseStruct() (#13152)
Summary: `OptionTypeInfo::ParseStruct()` was not honoring `config_options.ignore_unknown_options` when unknown properties are found in the serialized string. This caused a compatibility issue in Remote Compaction. When the worker was updated with RocksDB 9.9, the remote worker started including a new table property, `newest_key_time` (added in PR https://github.com/facebook/rocksdb/issues/13083), in the compaction output files. However, parsing that table property in the serialized compaction result from the primary (running with `9.8`) was returning a non-ok status, even though `config_options.ignore_unknown_options` was `true`. In this fix, we will ignore unused properties if `config_options.ignore_unknown_options` is set to true. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13152 Test Plan: Unit Test Added Reviewed By: archang19 Differential Revision: D66374541 Pulled By: jaykorean fbshipit-source-id: 78fd8309909279390438c247c4d390bbee4fa914
This commit is contained in:
parent
26b480609c
commit
e9b15738d9
|
@ -995,7 +995,8 @@ Status OptionTypeInfo::ParseStruct(
|
||||||
std::unordered_map<std::string, std::string> unused;
|
std::unordered_map<std::string, std::string> unused;
|
||||||
status =
|
status =
|
||||||
ParseType(config_options, opt_value, *struct_map, opt_addr, &unused);
|
ParseType(config_options, opt_value, *struct_map, opt_addr, &unused);
|
||||||
if (status.ok() && !unused.empty()) {
|
if (status.ok() && !unused.empty() &&
|
||||||
|
!config_options.ignore_unknown_options) {
|
||||||
status = Status::InvalidArgument(
|
status = Status::InvalidArgument(
|
||||||
"Unrecognized option", struct_name + "." + unused.begin()->first);
|
"Unrecognized option", struct_name + "." + unused.begin()->first);
|
||||||
}
|
}
|
||||||
|
@ -1006,7 +1007,7 @@ Status OptionTypeInfo::ParseStruct(
|
||||||
Find(opt_name.substr(struct_name.size() + 1), *struct_map, &elem_name);
|
Find(opt_name.substr(struct_name.size() + 1), *struct_map, &elem_name);
|
||||||
if (opt_info != nullptr) {
|
if (opt_info != nullptr) {
|
||||||
status = opt_info->Parse(config_options, elem_name, opt_value, opt_addr);
|
status = opt_info->Parse(config_options, elem_name, opt_value, opt_addr);
|
||||||
} else {
|
} else if (!config_options.ignore_unknown_options) {
|
||||||
status = Status::InvalidArgument("Unrecognized option", opt_name);
|
status = Status::InvalidArgument("Unrecognized option", opt_name);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
@ -1015,7 +1016,7 @@ Status OptionTypeInfo::ParseStruct(
|
||||||
const auto opt_info = Find(opt_name, *struct_map, &elem_name);
|
const auto opt_info = Find(opt_name, *struct_map, &elem_name);
|
||||||
if (opt_info != nullptr) {
|
if (opt_info != nullptr) {
|
||||||
status = opt_info->Parse(config_options, elem_name, opt_value, opt_addr);
|
status = opt_info->Parse(config_options, elem_name, opt_value, opt_addr);
|
||||||
} else {
|
} else if (!config_options.ignore_unknown_options) {
|
||||||
status = Status::InvalidArgument("Unrecognized option",
|
status = Status::InvalidArgument("Unrecognized option",
|
||||||
struct_name + "." + opt_name);
|
struct_name + "." + opt_name);
|
||||||
}
|
}
|
||||||
|
|
|
@ -832,6 +832,21 @@ TEST_F(OptionsTest, CompressionOptionsFromString) {
|
||||||
ASSERT_OK(GetColumnFamilyOptionsFromString(ignore, ColumnFamilyOptions(),
|
ASSERT_OK(GetColumnFamilyOptionsFromString(ignore, ColumnFamilyOptions(),
|
||||||
"compression_opts.unknown=bad",
|
"compression_opts.unknown=bad",
|
||||||
&base_cf_opt));
|
&base_cf_opt));
|
||||||
|
|
||||||
|
// Test with some additional values added
|
||||||
|
config_options.ignore_unknown_options = false;
|
||||||
|
ASSERT_NOK(
|
||||||
|
GetColumnFamilyOptionsFromString(config_options, ColumnFamilyOptions(),
|
||||||
|
"new_unknown_field=whatever; "
|
||||||
|
"bottommost_compression_opts=4:5:6:7",
|
||||||
|
&base_cf_opt));
|
||||||
|
// Test Ignoring Unknown Options
|
||||||
|
config_options.ignore_unknown_options = true;
|
||||||
|
ASSERT_OK(
|
||||||
|
GetColumnFamilyOptionsFromString(config_options, ColumnFamilyOptions(),
|
||||||
|
"new_unknown_field=whatever; "
|
||||||
|
"bottommost_compression_opts=4:5:6:7",
|
||||||
|
&base_cf_opt));
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST_F(OptionsTest, OldInterfaceTest) {
|
TEST_F(OptionsTest, OldInterfaceTest) {
|
||||||
|
|
|
@ -0,0 +1 @@
|
||||||
|
Honor ConfigOptions.ignore_unknown_options in ParseStruct()
|
Loading…
Reference in New Issue