Allow upgrades from nullptr to some merge operator

Summary:
Currently, RocksDB does not allow reopening a preexisting DB with no merge operator defined, with a merge operator defined. This means that if a DB ever want to add a merge operator, there's no way to do so currently.

Fix this by adding a new verification type `kByNameAllowFromNull` which will allow old values to be nullptr, and new values to be non-nullptr.
Closes https://github.com/facebook/rocksdb/pull/2958

Differential Revision: D5961131

Pulled By: lth

fbshipit-source-id: 06179bebd0d90db3d43690b5eb7345e2d5bab1eb
main
Manuel Ung 7 years ago committed by Facebook Github Bot
parent 5b2cb64bfb
commit 88ed1f6ea6
  1. 1
      options/options_helper.cc
  2. 11
      options/options_helper.h
  3. 7
      options/options_parser.cc
  4. 18
      options/options_test.cc
  5. 2
      table/block_based_table_factory.cc
  6. 2
      table/plain_table_factory.cc

@ -787,6 +787,7 @@ Status ParseColumnFamilyOption(const std::string& name,
switch (opt_info.verification) { switch (opt_info.verification) {
case OptionVerificationType::kByName: case OptionVerificationType::kByName:
case OptionVerificationType::kByNameAllowNull: case OptionVerificationType::kByNameAllowNull:
case OptionVerificationType::kByNameAllowFromNull:
return Status::NotSupported( return Status::NotSupported(
"Deserializing the specified CF option " + name + "Deserializing the specified CF option " + name +
" is not supported"); " is not supported");

@ -98,11 +98,13 @@ enum class OptionVerificationType {
// based on it's name. // based on it's name.
kByNameAllowNull, // Same as kByName, but it also allows the case kByNameAllowNull, // Same as kByName, but it also allows the case
// where one of them is a nullptr. // where one of them is a nullptr.
kByNameAllowFromNull, // Same as kByName, but it also allows the case
// where the old option is nullptr.
kDeprecated // The option is no longer used in rocksdb. The RocksDB kDeprecated // The option is no longer used in rocksdb. The RocksDB
// OptionsParser will still accept this option if it // OptionsParser will still accept this option if it
// happen to exists in some Options file. However, the // happen to exists in some Options file. However,
// parser will not include it in serialization and // the parser will not include it in serialization
// verification processes. // and verification processes.
}; };
// A struct for storing constant option information such as option name, // A struct for storing constant option information such as option name,
@ -575,7 +577,8 @@ static std::unordered_map<std::string, OptionTypeInfo> cf_options_type_info = {
false, 0}}, false, 0}},
{"merge_operator", {"merge_operator",
{offset_of(&ColumnFamilyOptions::merge_operator), {offset_of(&ColumnFamilyOptions::merge_operator),
OptionType::kMergeOperator, OptionVerificationType::kByName, false, 0}}, OptionType::kMergeOperator, OptionVerificationType::kByNameAllowFromNull,
false, 0}},
{"compaction_style", {"compaction_style",
{offset_of(&ColumnFamilyOptions::compaction_style), {offset_of(&ColumnFamilyOptions::compaction_style),
OptionType::kCompactionStyle, OptionVerificationType::kNormal, false, 0}}, OptionType::kCompactionStyle, OptionVerificationType::kNormal, false, 0}},

@ -589,6 +589,8 @@ bool AreEqualOptions(
*reinterpret_cast<const InfoLogLevel*>(offset2)); *reinterpret_cast<const InfoLogLevel*>(offset2));
default: default:
if (type_info.verification == OptionVerificationType::kByName || if (type_info.verification == OptionVerificationType::kByName ||
type_info.verification ==
OptionVerificationType::kByNameAllowFromNull ||
type_info.verification == OptionVerificationType::kByNameAllowNull) { type_info.verification == OptionVerificationType::kByNameAllowNull) {
std::string value1; std::string value1;
bool result = bool result =
@ -608,6 +610,11 @@ bool AreEqualOptions(
if (iter->second == kNullptrString || value1 == kNullptrString) { if (iter->second == kNullptrString || value1 == kNullptrString) {
return true; return true;
} }
} else if (type_info.verification ==
OptionVerificationType::kByNameAllowFromNull) {
if (iter->second == kNullptrString) {
return true;
}
} }
return (value1 == iter->second); return (value1 == iter->second);
} }

@ -1544,6 +1544,15 @@ TEST_F(OptionsSanityCheckTest, SanityCheck) {
// merge_operator // merge_operator
{ {
// Test when going from nullptr -> merge operator
opts.merge_operator.reset(test::RandomMergeOperator(&rnd));
ASSERT_OK(SanityCheckCFOptions(opts, kSanityLevelLooselyCompatible));
ASSERT_OK(SanityCheckCFOptions(opts, kSanityLevelNone));
// persist the change
ASSERT_OK(PersistCFOptions(opts));
ASSERT_OK(SanityCheckCFOptions(opts, kSanityLevelExactMatch));
for (int test = 0; test < 5; ++test) { for (int test = 0; test < 5; ++test) {
// change the merge operator // change the merge operator
opts.merge_operator.reset(test::RandomMergeOperator(&rnd)); opts.merge_operator.reset(test::RandomMergeOperator(&rnd));
@ -1554,6 +1563,15 @@ TEST_F(OptionsSanityCheckTest, SanityCheck) {
ASSERT_OK(PersistCFOptions(opts)); ASSERT_OK(PersistCFOptions(opts));
ASSERT_OK(SanityCheckCFOptions(opts, kSanityLevelExactMatch)); ASSERT_OK(SanityCheckCFOptions(opts, kSanityLevelExactMatch));
} }
// Test when going from merge operator -> nullptr
opts.merge_operator = nullptr;
ASSERT_NOK(SanityCheckCFOptions(opts, kSanityLevelLooselyCompatible));
ASSERT_OK(SanityCheckCFOptions(opts, kSanityLevelNone));
// persist the change
ASSERT_OK(PersistCFOptions(opts));
ASSERT_OK(SanityCheckCFOptions(opts, kSanityLevelExactMatch));
} }
// compaction_filter // compaction_filter

@ -346,6 +346,8 @@ Status GetBlockBasedTableOptionsFromMap(
(iter->second.verification != OptionVerificationType::kByName && (iter->second.verification != OptionVerificationType::kByName &&
iter->second.verification != iter->second.verification !=
OptionVerificationType::kByNameAllowNull && OptionVerificationType::kByNameAllowNull &&
iter->second.verification !=
OptionVerificationType::kByNameAllowFromNull &&
iter->second.verification != OptionVerificationType::kDeprecated)) { iter->second.verification != OptionVerificationType::kDeprecated)) {
// Restore "new_options" to the default "base_options". // Restore "new_options" to the default "base_options".
*new_table_options = table_options; *new_table_options = table_options;

@ -210,6 +210,8 @@ Status GetPlainTableOptionsFromMap(
(iter->second.verification != OptionVerificationType::kByName && (iter->second.verification != OptionVerificationType::kByName &&
iter->second.verification != iter->second.verification !=
OptionVerificationType::kByNameAllowNull && OptionVerificationType::kByNameAllowNull &&
iter->second.verification !=
OptionVerificationType::kByNameAllowFromNull &&
iter->second.verification != OptionVerificationType::kDeprecated)) { iter->second.verification != OptionVerificationType::kDeprecated)) {
// Restore "new_options" to the default "base_options". // Restore "new_options" to the default "base_options".
*new_table_options = table_options; *new_table_options = table_options;

Loading…
Cancel
Save