Prevent an incompatible combination of options (#6254)

Summary:
allow_concurrent_memtable_write is incompatible with non-zero max_successive_merges. Although we check this at runtime, we currently don't prevent the user from setting this combination in options. This has led to stress tests to fail with this combination is tried in ::SetOptions. The patch fixes that.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6254

Differential Revision: D19265819

Pulled By: maysamyabandeh

fbshipit-source-id: 47f2e2dc26fe0972c7152f4da15dadb9703f1179
This commit is contained in:
Maysam Yabandeh 2020-01-02 16:13:30 -08:00 committed by Facebook Github Bot
parent 83108d23e8
commit 48a678b7c9
7 changed files with 16 additions and 2 deletions

View File

@ -160,6 +160,11 @@ Status CheckConcurrentWritesSupported(const ColumnFamilyOptions& cf_options) {
return Status::InvalidArgument(
"Memtable doesn't concurrent writes (allow_concurrent_memtable_write)");
}
if (cf_options.max_successive_merges != 0) {
return Status::InvalidArgument(
"max_successive_merges > 0 is incompatible "
"with concurrent writes (allow_concurrent_memtable_write)");
}
return Status::OK();
}

View File

@ -150,6 +150,8 @@ TEST_F(DBMergeOperatorTest, MergeErrorOnWrite) {
options.create_if_missing = true;
options.merge_operator.reset(new TestPutOperator());
options.max_successive_merges = 3;
// allow_concurrent_memtable_write is incompatible with max_successive_merges
options.allow_concurrent_memtable_write = false;
options.env = env_;
Reopen(options);
ASSERT_OK(Merge("k1", "v1"));

View File

@ -153,6 +153,8 @@ TEST_F(DBTest2, MaxSuccessiveMergesChangeWithDBRecovery) {
options.create_if_missing = true;
options.statistics = rocksdb::CreateDBStatistics();
options.max_successive_merges = 3;
// allow_concurrent_memtable_write is incompatible with max_successive_merges
options.allow_concurrent_memtable_write = false;
options.merge_operator = MergeOperators::CreatePutOperator();
options.disable_auto_compactions = true;
DestroyAndReopen(options);

View File

@ -78,6 +78,9 @@ std::shared_ptr<DB> OpenDb(const std::string& dbname, const bool ttl = false,
options.create_if_missing = true;
options.merge_operator = std::make_shared<CountMergeOperator>();
options.max_successive_merges = max_successive_merges;
if (max_successive_merges > 0) {
options.allow_concurrent_memtable_write = false;
}
Status s;
DestroyDB(dbname, Options());
// DBWithTTL is not supported in ROCKSDB_LITE

View File

@ -1946,6 +1946,7 @@ void StressTest::Open() {
}
s = TransactionDB::Open(options_, txn_db_options, FLAGS_db,
cf_descriptors, &column_families_, &txn_db_);
assert(s.ok());
db_ = txn_db_;
// after a crash, rollback to commit recovered transactions
std::vector<Transaction*> trans;

View File

@ -351,7 +351,8 @@ void RandomInitCFOptions(ColumnFamilyOptions* cf_opt, DBOptions& db_options,
// size_t options
cf_opt->arena_block_size = rnd->Uniform(10000);
cf_opt->inplace_update_num_locks = rnd->Uniform(10000);
cf_opt->max_successive_merges = rnd->Uniform(10000);
cf_opt->max_successive_merges =
db_options.allow_concurrent_memtable_write ? 0 : rnd->Uniform(10000);
cf_opt->memtable_huge_page_size = rnd->Uniform(10000);
cf_opt->write_buffer_size = rnd->Uniform(10000);

View File

@ -5820,7 +5820,7 @@ TEST_P(TransactionTest, DuplicateKeys) {
} // do_rollback
} // do_prepare
if (!options.unordered_write) {
if (!options.unordered_write && !options.allow_concurrent_memtable_write) {
// Also test with max_successive_merges > 0. max_successive_merges will not
// affect our algorithm for duplicate key insertion but we add the test to
// verify that.