andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert LoadLatestOptions handling of ignore_unknown_options if versions differ (#7612)

Browse Source 
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7612

Reviewed By: zhichao-cao

Differential Revision: D24627054

Pulled By: riversand963

fbshipit-source-id: 451b4da742e3e84c7442bc7cc4959d39089b89d0
This commit is contained in:
mrambacher 2020-10-29 13:45:17 -07:00 committed by Andrew Kryczka
parent 91f3b72ebc
commit e5686db330
3 changed files with 35 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
options/options_parser.cc
View File

@ -289,13 +289,13 @@ Status RocksDBOptionsParser::Parse(const ConfigOptions& config_options_in,
return s; return s;
} }
// If the option file is newer than the current version // If the option file is not generated by a higher minor version,
// there may be unknown options. // there shouldn't be any unknown option.
if (!config_options.ignore_unknown_options && if (config_options.ignore_unknown_options &&
section == kOptionSectionVersion) { section == kOptionSectionVersion) {
if (db_version[0] > ROCKSDB_MAJOR || if (db_version[0] < ROCKSDB_MAJOR || (db_version[0] == ROCKSDB_MAJOR &&
(db_version[0] == ROCKSDB_MAJOR && db_version[1] > ROCKSDB_MINOR)) { db_version[1] <= ROCKSDB_MINOR)) {
config_options.ignore_unknown_options = true; config_options.ignore_unknown_options = false;
} }
} }

8
options/options_test.cc
View File

@ -2659,15 +2659,15 @@ TEST_F(OptionsParserTest, IgnoreUnknownOptions) {
} }
ASSERT_OK(env_->WriteToNewFile(kTestFileName, options_file_content)); ASSERT_OK(env_->WriteToNewFile(kTestFileName, options_file_content));
RocksDBOptionsParser parser; RocksDBOptionsParser parser;
ASSERT_OK(parser.Parse(kTestFileName, fs_.get(), true, ASSERT_NOK(parser.Parse(kTestFileName, fs_.get(), false,
4096 /* readahead_size */)); 4096 /* readahead_size */));
if (should_ignore) { if (should_ignore) {
ASSERT_OK(parser.Parse(kTestFileName, fs_.get(), ASSERT_OK(parser.Parse(kTestFileName, fs_.get(),
false /* ignore_unknown_options */, true /* ignore_unknown_options */,
4096 /* readahead_size */)); 4096 /* readahead_size */));
} else { } else {
ASSERT_NOK(parser.Parse(kTestFileName, fs_.get(), ASSERT_NOK(parser.Parse(kTestFileName, fs_.get(),
false /* ignore_unknown_options */, true /* ignore_unknown_options */,
4096 /* readahead_size */)); 4096 /* readahead_size */));
} }
} }

34
utilities/options/options_util_test.cc
View File

@ -552,8 +552,10 @@ TEST_F(OptionsUtilTest, BadLatestOptions) {
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs); s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s); ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument()); ASSERT_TRUE(s.IsInvalidArgument());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); // Even though ignore_unknown_options=true, we still return an error...
s = LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument());
// Write an options file for a previous minor release with an unknown CF // Write an options file for a previous minor release with an unknown CF
// Option // Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0002", ROCKSDB_MAJOR, WriteOptionsFile(options.env, dbname_, "OPTIONS-0002", ROCKSDB_MAJOR,
@ -561,7 +563,10 @@ TEST_F(OptionsUtilTest, BadLatestOptions) {
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs); s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s); ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument()); ASSERT_TRUE(s.IsInvalidArgument());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); // Even though ignore_unknown_options=true, we still return an error...
s = LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument());
// Write an options file for the current release with an unknown DB Option // Write an options file for the current release with an unknown DB Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0003", ROCKSDB_MAJOR, WriteOptionsFile(options.env, dbname_, "OPTIONS-0003", ROCKSDB_MAJOR,
@ -569,7 +574,10 @@ TEST_F(OptionsUtilTest, BadLatestOptions) {
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs); s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s); ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument()); ASSERT_TRUE(s.IsInvalidArgument());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); // Even though ignore_unknown_options=true, we still return an error...
s = LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument());
// Write an options file for the current release with an unknown CF Option // Write an options file for the current release with an unknown CF Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0004", ROCKSDB_MAJOR, WriteOptionsFile(options.env, dbname_, "OPTIONS-0004", ROCKSDB_MAJOR,
@ -577,7 +585,10 @@ TEST_F(OptionsUtilTest, BadLatestOptions) {
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs); s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s); ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument()); ASSERT_TRUE(s.IsInvalidArgument());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); // Even though ignore_unknown_options=true, we still return an error...
s = LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument());
// Write an options file for the current release with an invalid DB Option // Write an options file for the current release with an invalid DB Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0005", ROCKSDB_MAJOR, WriteOptionsFile(options.env, dbname_, "OPTIONS-0005", ROCKSDB_MAJOR,
@ -585,24 +596,29 @@ TEST_F(OptionsUtilTest, BadLatestOptions) {
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs); s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s); ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument()); ASSERT_TRUE(s.IsInvalidArgument());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); // Even though ignore_unknown_options=true, we still return an error...
s = LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument());
// Write an options file for the next release with an invalid DB Option // Write an options file for the next release with an invalid DB Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0006", ROCKSDB_MAJOR, WriteOptionsFile(options.env, dbname_, "OPTIONS-0006", ROCKSDB_MAJOR,
ROCKSDB_MINOR + 1, "create_if_missing=hello", ""); ROCKSDB_MINOR + 1, "create_if_missing=hello", "");
ASSERT_OK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs)); ASSERT_NOK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));
// Write an options file for the next release with an unknown DB Option // Write an options file for the next release with an unknown DB Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0007", ROCKSDB_MAJOR, WriteOptionsFile(options.env, dbname_, "OPTIONS-0007", ROCKSDB_MAJOR,
ROCKSDB_MINOR + 1, "unknown_db_opt=true", ""); ROCKSDB_MINOR + 1, "unknown_db_opt=true", "");
ASSERT_OK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs)); ASSERT_NOK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));
// Ignore the errors for future releases when ignore_unknown_options=true
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));
// Write an options file for the next major release with an unknown CF Option // Write an options file for the next major release with an unknown CF Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0008", ROCKSDB_MAJOR + 1, WriteOptionsFile(options.env, dbname_, "OPTIONS-0008", ROCKSDB_MAJOR + 1,
ROCKSDB_MINOR, "", "unknown_cf_opt=true"); ROCKSDB_MINOR, "", "unknown_cf_opt=true");
ASSERT_OK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs)); ASSERT_NOK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));
// Ignore the errors for future releases when ignore_unknown_options=true
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs)); ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));
} }
} // namespace ROCKSDB_NAMESPACE } // namespace ROCKSDB_NAMESPACE