Skip to content

Commit

Permalink
Test for LoadLatestOptions (facebook#7554)
Browse files Browse the repository at this point in the history 
Summary:
Make LoadLatestOptions return PathNotFound if the options file does not exist.  Added tests for the LoadOptions related methods.

Pull Request resolved: facebook#7554

Reviewed By: akankshamahajan15

Differential Revision: D24298985

Pulled By: zhichao-cao

fbshipit-source-id: c9ae3cb12fc4a5bbef07743e1c1300f98a2441b3
  • Loading branch information
@mrambacher @codingrhythm
mrambacher authored and codingrhythm committed Mar 5, 2021
1 parent 48c130c commit bc502cf
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 22 deletions.
1 change: 1 addition & 0 deletions db/db_impl/db_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,7 @@ Status DBImpl::SetOptions(
MutableCFOptions new_options;
Status s;
Status persist_options_status;
persist_options_status.PermitUncheckedError(); // Allow uninitialized access
SuperVersionContext sv_context(/* create_superversion */ true);
{
auto db_options = GetDBOptions();
Expand Down
6 changes: 4 additions & 2 deletions memtable/write_buffer_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ void WriteBufferManager::ReserveMemWithCache(size_t mem) {
// Expand size by at least 256KB.
// Add a dummy record to the cache
Cache::Handle* handle = nullptr;
cache_rep_->cache_->Insert(cache_rep_->GetNextCacheKey(), nullptr,
kSizeDummyEntry, nullptr, &handle);
Status s =
cache_rep_->cache_->Insert(cache_rep_->GetNextCacheKey(), nullptr,
kSizeDummyEntry, nullptr, &handle);
s.PermitUncheckedError(); // TODO: What to do on error?
// We keep the handle even if insertion fails and a null handle is
// returned, so that when memory shrinks, we don't release extra
// entries from cache.
Expand Down
12 changes: 6 additions & 6 deletions options/options_parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,13 @@ Status RocksDBOptionsParser::Parse(const ConfigOptions& config_options_in,
return s;
}

// If the option file is not generated by a higher minor version,
// there shouldn't be any unknown option.
if (config_options.ignore_unknown_options &&
// If the option file is newer than the current version
// there may be unknown options.
if (!config_options.ignore_unknown_options &&
section == kOptionSectionVersion) {
if (db_version[0] < ROCKSDB_MAJOR || (db_version[0] == ROCKSDB_MAJOR &&
db_version[1] <= ROCKSDB_MINOR)) {
config_options.ignore_unknown_options = false;
if (db_version[0] > ROCKSDB_MAJOR ||
(db_version[0] == ROCKSDB_MAJOR && db_version[1] > ROCKSDB_MINOR)) {
config_options.ignore_unknown_options = true;
}
}

Expand Down
8 changes: 4 additions & 4 deletions options/options_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2633,15 +2633,15 @@ TEST_F(OptionsParserTest, IgnoreUnknownOptions) {
}
ASSERT_OK(env_->WriteToNewFile(kTestFileName, options_file_content));
RocksDBOptionsParser parser;
ASSERT_NOK(parser.Parse(kTestFileName, fs_.get(), false,
4096 /* readahead_size */));
ASSERT_OK(parser.Parse(kTestFileName, fs_.get(), true,
4096 /* readahead_size */));
if (should_ignore) {
ASSERT_OK(parser.Parse(kTestFileName, fs_.get(),
true /* ignore_unknown_options */,
false /* ignore_unknown_options */,
4096 /* readahead_size */));
} else {
ASSERT_NOK(parser.Parse(kTestFileName, fs_.get(),
true /* ignore_unknown_options */,
false /* ignore_unknown_options */,
4096 /* readahead_size */));
}
}
Expand Down
8 changes: 6 additions & 2 deletions utilities/options/options_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ Status GetLatestOptionsFileName(const std::string& dbpath,
uint64_t latest_time_stamp = 0;
std::vector<std::string> file_names;
s = env->GetChildren(dbpath, &file_names);
if (!s.ok()) {
if (s.IsNotFound()) {
return Status::PathNotFound("No options files found in the DB directory.",
dbpath);
} else if (!s.ok()) {
return s;
}
for (auto& file_name : file_names) {
Expand All @@ -79,7 +82,8 @@ Status GetLatestOptionsFileName(const std::string& dbpath,
}
}
if (latest_file_name.size() == 0) {
return Status::NotFound("No options files found in the DB directory.");
return Status::PathNotFound("No options files found in the DB directory.",
dbpath);
}
*options_file_name = latest_file_name;
return Status::OK();
Expand Down
253 changes: 245 additions & 8 deletions utilities/options/options_util_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include <cinttypes>
#include <unordered_map>

#include "env/mock_env.h"
#include "file/filename.h"
#include "options/options_parser.h"
#include "rocksdb/convenience.h"
#include "rocksdb/db.h"
Expand All @@ -31,14 +33,12 @@ namespace ROCKSDB_NAMESPACE {
class OptionsUtilTest : public testing::Test {
public:
OptionsUtilTest() : rnd_(0xFB) {
env_.reset(new test::StringEnv(Env::Default()));
fs_.reset(new LegacyFileSystemWrapper(env_.get()));
env_.reset(NewMemEnv(Env::Default()));
dbname_ = test::PerThreadDBPath("options_util_test");
}

protected:
std::unique_ptr<test::StringEnv> env_;
std::unique_ptr<LegacyFileSystemWrapper> fs_;
std::unique_ptr<Env> env_;
std::string dbname_;
Random rnd_;
};
Expand All @@ -58,8 +58,8 @@ TEST_F(OptionsUtilTest, SaveAndLoad) {
}

const std::string kFileName = "OPTIONS-123456";
ASSERT_OK(
PersistRocksDBOptions(db_opt, cf_names, cf_opts, kFileName, fs_.get()));
ASSERT_OK(PersistRocksDBOptions(db_opt, cf_names, cf_opts, kFileName,
env_->GetFileSystem().get()));

DBOptions loaded_db_opt;
std::vector<ColumnFamilyDescriptor> loaded_cf_descs;
Expand All @@ -85,6 +85,7 @@ TEST_F(OptionsUtilTest, SaveAndLoad) {
exact, cf_opts[i], loaded_cf_descs[i].options));
}

DestroyDB(dbname_, Options(db_opt, cf_opts[0]));
for (size_t i = 0; i < kCFCount; ++i) {
if (cf_opts[i].compaction_filter) {
delete cf_opts[i].compaction_filter;
Expand Down Expand Up @@ -121,8 +122,8 @@ TEST_F(OptionsUtilTest, SaveAndLoadWithCacheCheck) {
cf_names.push_back("cf_plain_table_sample");
// Saving DB in file
const std::string kFileName = "OPTIONS-LOAD_CACHE_123456";
ASSERT_OK(
PersistRocksDBOptions(db_opt, cf_names, cf_opts, kFileName, fs_.get()));
ASSERT_OK(PersistRocksDBOptions(db_opt, cf_names, cf_opts, kFileName,
env_->GetFileSystem().get()));
DBOptions loaded_db_opt;
std::vector<ColumnFamilyDescriptor> loaded_cf_descs;

Expand Down Expand Up @@ -154,6 +155,7 @@ TEST_F(OptionsUtilTest, SaveAndLoadWithCacheCheck) {
ASSERT_EQ(loaded_bbt_opt->block_cache.get(), cache.get());
}
}
DestroyDB(dbname_, Options(loaded_db_opt, cf_opts[0]));
}

namespace {
Expand Down Expand Up @@ -359,8 +361,243 @@ TEST_F(OptionsUtilTest, SanityCheck) {
ASSERT_OK(
CheckOptionsCompatibility(config_options, dbname_, db_opt, cf_descs));
}
DestroyDB(dbname_, Options(db_opt, cf_descs[0].options));
}

TEST_F(OptionsUtilTest, LatestOptionsNotFound) {
std::unique_ptr<Env> env(NewMemEnv(Env::Default()));
Status s;
Options options;
ConfigOptions config_opts;
std::vector<ColumnFamilyDescriptor> cf_descs;

options.env = env.get();
options.create_if_missing = true;
config_opts.env = options.env;
config_opts.ignore_unknown_options = false;

std::vector<std::string> children;

std::string options_file_name;
DestroyDB(dbname_, options);
// First, test where the db directory does not exist
ASSERT_NOK(options.env->GetChildren(dbname_, &children));

s = GetLatestOptionsFileName(dbname_, options.env, &options_file_name);
ASSERT_TRUE(s.IsPathNotFound());

s = LoadLatestOptions(dbname_, options.env, &options, &cf_descs);
ASSERT_TRUE(s.IsPathNotFound());

s = LoadLatestOptions(config_opts, dbname_, &options, &cf_descs);
ASSERT_TRUE(s.IsPathNotFound());

s = GetLatestOptionsFileName(dbname_, options.env, &options_file_name);
ASSERT_TRUE(s.IsPathNotFound());

// Second, test where the db directory exists but is empty
ASSERT_OK(options.env->CreateDir(dbname_));

s = GetLatestOptionsFileName(dbname_, options.env, &options_file_name);
ASSERT_TRUE(s.IsPathNotFound());

s = LoadLatestOptions(dbname_, options.env, &options, &cf_descs);
ASSERT_TRUE(s.IsPathNotFound());

// Finally, test where a file exists but is not an "Options" file
std::unique_ptr<WritableFile> file;
ASSERT_OK(
options.env->NewWritableFile(dbname_ + "/temp.txt", &file, EnvOptions()));
ASSERT_OK(file->Close());
s = GetLatestOptionsFileName(dbname_, options.env, &options_file_name);
ASSERT_TRUE(s.IsPathNotFound());

s = LoadLatestOptions(config_opts, dbname_, &options, &cf_descs);
ASSERT_TRUE(s.IsPathNotFound());
ASSERT_OK(options.env->DeleteFile(dbname_ + "/temp.txt"));
ASSERT_OK(options.env->DeleteDir(dbname_));
}

TEST_F(OptionsUtilTest, LoadLatestOptions) {
Options options;
options.OptimizeForSmallDb();
ColumnFamilyDescriptor cf_desc;
ConfigOptions config_opts;
DBOptions db_opts;
std::vector<ColumnFamilyDescriptor> cf_descs;
std::vector<ColumnFamilyHandle*> handles;
DB* db;
options.create_if_missing = true;

DestroyDB(dbname_, options);

cf_descs.emplace_back();
cf_descs.back().name = kDefaultColumnFamilyName;
cf_descs.back().options.table_factory.reset(NewBlockBasedTableFactory());
cf_descs.emplace_back();
cf_descs.back().name = "Plain";
cf_descs.back().options.table_factory.reset(NewPlainTableFactory());
db_opts.create_missing_column_families = true;
db_opts.create_if_missing = true;

// open and persist the options
ASSERT_OK(DB::Open(db_opts, dbname_, cf_descs, &handles, &db));

std::string options_file_name;
std::string new_options_file;

ASSERT_OK(GetLatestOptionsFileName(dbname_, options.env, &options_file_name));
ASSERT_OK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));
ASSERT_EQ(cf_descs.size(), 2U);
ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(config_opts,
db->GetDBOptions(), db_opts));
ASSERT_OK(handles[0]->GetDescriptor(&cf_desc));
ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(config_opts, cf_desc.options,
cf_descs[0].options));
ASSERT_OK(handles[1]->GetDescriptor(&cf_desc));
ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(config_opts, cf_desc.options,
cf_descs[1].options));

// Now change some of the DBOptions
ASSERT_OK(db->SetDBOptions(
{{"delayed_write_rate", "1234"}, {"bytes_per_sync", "32768"}}));
ASSERT_OK(GetLatestOptionsFileName(dbname_, options.env, &new_options_file));
ASSERT_NE(options_file_name, new_options_file);
ASSERT_OK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));
ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(config_opts,
db->GetDBOptions(), db_opts));
options_file_name = new_options_file;

// Now change some of the ColumnFamilyOptions
ASSERT_OK(db->SetOptions(handles[1], {{"write_buffer_size", "32768"}}));
ASSERT_OK(GetLatestOptionsFileName(dbname_, options.env, &new_options_file));
ASSERT_NE(options_file_name, new_options_file);
ASSERT_OK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));
ASSERT_OK(RocksDBOptionsParser::VerifyDBOptions(config_opts,
db->GetDBOptions(), db_opts));
ASSERT_OK(handles[0]->GetDescriptor(&cf_desc));
ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(config_opts, cf_desc.options,
cf_descs[0].options));
ASSERT_OK(handles[1]->GetDescriptor(&cf_desc));
ASSERT_OK(RocksDBOptionsParser::VerifyCFOptions(config_opts, cf_desc.options,
cf_descs[1].options));

// close the db
for (auto* handle : handles) {
delete handle;
}
delete db;
DestroyDB(dbname_, options, cf_descs);
}

static void WriteOptionsFile(Env* env, const std::string& path,
const std::string& options_file, int major,
int minor, const std::string& db_opts,
const std::string& cf_opts) {
std::string options_file_header =
"\n"
"[Version]\n"
" rocksdb_version=" +
ToString(major) + "." + ToString(minor) +
".0\n"
" options_file_version=1\n";

std::unique_ptr<WritableFile> wf;
ASSERT_OK(env->NewWritableFile(path + "/" + options_file, &wf, EnvOptions()));
ASSERT_OK(
wf->Append(options_file_header + "[ DBOptions ]\n" + db_opts + "\n"));
ASSERT_OK(wf->Append(
"[CFOptions \"default\"] # column family must be specified\n" +
cf_opts + "\n"));
ASSERT_OK(wf->Close());

std::string latest_options_file;
ASSERT_OK(GetLatestOptionsFileName(path, env, &latest_options_file));
ASSERT_EQ(latest_options_file, options_file);
}

TEST_F(OptionsUtilTest, BadLatestOptions) {
Status s;
ConfigOptions config_opts;
DBOptions db_opts;
std::vector<ColumnFamilyDescriptor> cf_descs;
Options options;
options.env = env_.get();
config_opts.env = env_.get();
config_opts.ignore_unknown_options = false;
config_opts.delimiter = "\n";

ConfigOptions ignore_opts = config_opts;
ignore_opts.ignore_unknown_options = true;

std::string options_file_name;

// Test where the db directory exists but is empty
ASSERT_OK(options.env->CreateDir(dbname_));
ASSERT_NOK(
GetLatestOptionsFileName(dbname_, options.env, &options_file_name));
ASSERT_NOK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));

// Write an options file for a previous major release with an unknown DB
// Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0001", ROCKSDB_MAJOR - 1,
ROCKSDB_MINOR, "unknown_db_opt=true", "");
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsNotFound());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));

// Write an options file for a previous minor release with an unknown CF
// Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0002", ROCKSDB_MAJOR,
ROCKSDB_MINOR - 1, "", "unknown_cf_opt=true");
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsNotFound());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));

// Write an options file for the current release with an unknown DB Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0003", ROCKSDB_MAJOR,
ROCKSDB_MINOR, "unknown_db_opt=true", "");
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsNotFound());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));

// Write an options file for the current release with an unknown CF Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0004", ROCKSDB_MAJOR,
ROCKSDB_MINOR, "", "unknown_cf_opt=true");
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsNotFound());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));

// Write an options file for the current release with an invalid DB Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0005", ROCKSDB_MAJOR,
ROCKSDB_MINOR, "create_if_missing=hello", "");
s = LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs);
ASSERT_NOK(s);
ASSERT_TRUE(s.IsInvalidArgument());
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));

// Write an options file for the next release with an invalid DB Option
WriteOptionsFile(options.env, dbname_, "OPTIONS-0006", ROCKSDB_MAJOR,
ROCKSDB_MINOR + 1, "create_if_missing=hello", "");
ASSERT_OK(LoadLatestOptions(config_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
WriteOptionsFile(options.env, dbname_, "OPTIONS-0007", ROCKSDB_MAJOR,
ROCKSDB_MINOR + 1, "unknown_db_opt=true", "");
ASSERT_OK(LoadLatestOptions(config_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
WriteOptionsFile(options.env, dbname_, "OPTIONS-0008", ROCKSDB_MAJOR + 1,
ROCKSDB_MINOR, "", "unknown_cf_opt=true");
ASSERT_OK(LoadLatestOptions(config_opts, dbname_, &db_opts, &cf_descs));
ASSERT_OK(LoadLatestOptions(ignore_opts, dbname_, &db_opts, &cf_descs));
}
} // namespace ROCKSDB_NAMESPACE

int main(int argc, char** argv) {
Expand Down

0 comments on commit bc502cf

Please sign in to comment.