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

Fix an issue with MemTableRepFactory::CreateFromString (#9273)

Browse Source 
Summary:
If ignore_unsupported_options=true, then it is possible for MemTableRepFactory::CreateFromString to succeed without setting a result (result=nullptr).  This would cause the original value to be overwritten with null and an error would be raised later when PrepareOptions is invoked.

Added unit test for this condition.  Will add (in another PR unless required by reviewers) comparable tests for all of the other Customizable classes.

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

Reviewed By: ltamasi

Differential Revision: D32990365

Pulled By: mrambacher

fbshipit-source-id: b150724c3f5ae7346357b3866244fd93466875c7
This commit is contained in:
mrambacher 2021-12-09 12:35:18 -08:00 committed by Levi Tamasi
parent 1217630bf3
commit f181158b95
3 changed files with 49 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
db/db_options_test.cc
View File

@ -266,6 +266,50 @@ TEST_F(DBOptionsTest, SetMutableTableOptions) {
ASSERT_EQ(c_bbto->block_restart_interval, 13);
}
TEST_F(DBOptionsTest, SetWithCustomMemTableFactory) {
class DummySkipListFactory : public SkipListFactory {
public:
static const char* kClassName() { return "DummySkipListFactory"; }
const char* Name() const override { return kClassName(); }
explicit DummySkipListFactory() : SkipListFactory(2) {}
};
{
// Verify the DummySkipList cannot be created
ConfigOptions config_options;
config_options.ignore_unsupported_options = false;
std::unique_ptr<MemTableRepFactory> factory;
ASSERT_NOK(MemTableRepFactory::CreateFromString(
config_options, DummySkipListFactory::kClassName(), &factory));
}
Options options;
options.create_if_missing = true;
// Try with fail_if_options_file_error=false/true to update the options
for (bool on_error : {false, true}) {
options.fail_if_options_file_error = on_error;
options.env = env_;
options.disable_auto_compactions = false;
options.memtable_factory.reset(new DummySkipListFactory());
Reopen(options);
ColumnFamilyHandle* cfh = dbfull()->DefaultColumnFamily();
ASSERT_OK(
dbfull()->SetOptions(cfh, {{"disable_auto_compactions", "true"}}));
ColumnFamilyDescriptor cfd;
ASSERT_OK(cfh->GetDescriptor(&cfd));
ASSERT_STREQ(cfd.options.memtable_factory->Name(),
DummySkipListFactory::kClassName());
ColumnFamilyHandle* test = nullptr;
ASSERT_OK(dbfull()->CreateColumnFamily(options, "test", &test));
ASSERT_OK(test->GetDescriptor(&cfd));
ASSERT_STREQ(cfd.options.memtable_factory->Name(),
DummySkipListFactory::kClassName());
ASSERT_OK(dbfull()->DropColumnFamily(test));
delete test;
}
}
TEST_F(DBOptionsTest, SetBytesPerSync) {
const size_t kValueSize = 1024 * 1024; // 1MB
Options options;

4
options/cf_options.cc
View File

@ -597,7 +597,7 @@ static std::unordered_map<std::string, OptionTypeInfo>
static_cast<std::shared_ptr<MemTableRepFactory>*>(addr);
Status s =
MemTableRepFactory::CreateFromString(opts, value, &factory);
if (s.ok()) {
if (factory && s.ok()) {
shared->reset(factory.release());
}
return s;
@ -613,7 +613,7 @@ static std::unordered_map<std::string, OptionTypeInfo>
static_cast<std::shared_ptr<MemTableRepFactory>*>(addr);
Status s =
MemTableRepFactory::CreateFromString(opts, value, &factory);
if (s.ok()) {
if (factory && s.ok()) {
shared->reset(factory.release());
}
return s;

6
options/configurable.cc
View File

@ -53,13 +53,13 @@ Status Configurable::PrepareOptions(const ConfigOptions& opts) {
opt_info.AsRawPointer<Configurable>(opt_iter.opt_ptr);
if (config != nullptr) {
status = config->PrepareOptions(opts);
if (!status.ok()) {
return status;
}
} else if (!opt_info.CanBeNull()) {
status = Status::NotFound("Missing configurable object",
map_iter.first);
}
if (!status.ok()) {
return status;
}
}
}
}