diff --git a/db/column_family.cc b/db/column_family.cc index 9e74df583..b2670cbdb 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -544,16 +544,16 @@ void ColumnFamilyData::ResetThreadLocalSuperVersions() { } } -bool ColumnFamilyData::SetOptions( +Status ColumnFamilyData::SetOptions( const std::unordered_map& options_map) { MutableCFOptions new_mutable_cf_options; - if (GetMutableOptionsFromStrings(mutable_cf_options_, options_map, - &new_mutable_cf_options)) { + Status s = GetMutableOptionsFromStrings(mutable_cf_options_, options_map, + &new_mutable_cf_options); + if (s.ok()) { mutable_cf_options_ = new_mutable_cf_options; mutable_cf_options_.RefreshDerivedOptions(ioptions_); - return true; } - return false; + return s; } ColumnFamilySet::ColumnFamilySet(const std::string& dbname, diff --git a/db/column_family.h b/db/column_family.h index b37b684fa..013c29615 100644 --- a/db/column_family.h +++ b/db/column_family.h @@ -187,7 +187,7 @@ class ColumnFamilyData { return &mutable_cf_options_; } // REQUIRES: DB mutex held - bool SetOptions( + Status SetOptions( const std::unordered_map& options_map); InternalStats* internal_stats() { return internal_stats_.get(); } diff --git a/db/db_impl.cc b/db/db_impl.cc index b5b77882e..2bbb3345f 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1142,23 +1142,23 @@ Status DBImpl::CompactRange(ColumnFamilyHandle* column_family, return s; } -bool DBImpl::SetOptions(ColumnFamilyHandle* column_family, +Status DBImpl::SetOptions(ColumnFamilyHandle* column_family, const std::unordered_map& options_map) { auto* cfd = reinterpret_cast(column_family)->cfd(); if (options_map.empty()) { Log(InfoLogLevel::WARN_LEVEL, db_options_.info_log, "SetOptions() on column family [%s], empty input", cfd->GetName().c_str()); - return false; + return Status::InvalidArgument("empty input"); } MutableCFOptions new_options; - bool succeed = false; + Status s; { MutexLock l(&mutex_); - if (cfd->SetOptions(options_map)) { + s = cfd->SetOptions(options_map); + if (s.ok()) { new_options = *cfd->GetLatestMutableCFOptions(); - succeed = true; } } @@ -1169,7 +1169,7 @@ bool DBImpl::SetOptions(ColumnFamilyHandle* column_family, Log(InfoLogLevel::INFO_LEVEL, db_options_.info_log, "%s: %s\n", o.first.c_str(), o.second.c_str()); } - if (succeed) { + if (s.ok()) { Log(InfoLogLevel::INFO_LEVEL, db_options_.info_log, "[%s] SetOptions succeeded", cfd->GetName().c_str()); @@ -1178,7 +1178,7 @@ bool DBImpl::SetOptions(ColumnFamilyHandle* column_family, Log(InfoLogLevel::WARN_LEVEL, db_options_.info_log, "[%s] SetOptions failed", cfd->GetName().c_str()); } - return succeed; + return s; } // return the same level if it cannot be moved diff --git a/db/db_impl.h b/db/db_impl.h index 5aa1eb8ed..8717dee90 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -115,7 +115,7 @@ class DBImpl : public DB { uint32_t target_path_id = 0); using DB::SetOptions; - bool SetOptions(ColumnFamilyHandle* column_family, + Status SetOptions(ColumnFamilyHandle* column_family, const std::unordered_map& options_map); using DB::NumberLevels; diff --git a/db/db_test.cc b/db/db_test.cc index 8f9972bd2..7aea863f8 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -8453,7 +8453,7 @@ TEST(DBTest, DynamicMemtableOptions) { ASSERT_EQ(NumTableFilesAtLevel(0), 0); // Increase buffer size - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"write_buffer_size", "131072"}, })); @@ -8495,7 +8495,7 @@ TEST(DBTest, DynamicMemtableOptions) { sleeping_task_low1.WaitUntilDone(); // Increase - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_write_buffer_number", "8"}, })); // Clean up memtable and L0 @@ -8514,7 +8514,7 @@ TEST(DBTest, DynamicMemtableOptions) { sleeping_task_low2.WaitUntilDone(); // Decrease - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_write_buffer_number", "4"}, })); // Clean up memtable and L0 @@ -8593,7 +8593,7 @@ TEST(DBTest, DynamicCompactionOptions) { // Writing to 64KB L0 files should trigger a compaction. Since these // 2 L0 files have the same key range, compaction merge them and should // result in 2 32KB L1 files. - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"level0_file_num_compaction_trigger", "2"}, {"target_file_size_base", std::to_string(k32KB) } })); @@ -8615,7 +8615,7 @@ TEST(DBTest, DynamicCompactionOptions) { // Increase level base size to 256KB and write enough data that will // fill L1 and L2. L1 size should be around 256KB while L2 size should be // around 256KB x 4. - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_bytes_for_level_base", std::to_string(k1MB) } })); @@ -8636,7 +8636,7 @@ TEST(DBTest, DynamicCompactionOptions) { // max_bytes_for_level_base. Now, reduce both mulitplier and level base, // After filling enough data that can fit in L1 - L3, we should see L1 size // reduces to 128KB from 256KB which was asserted previously. Same for L2. - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_bytes_for_level_multiplier", "2"}, {"max_bytes_for_level_base", std::to_string(k128KB) } })); @@ -8678,7 +8678,7 @@ TEST(DBTest, DynamicCompactionOptions) { // Now reduce level0_stop_writes_trigger to 6. Clear up memtables and L0. // Block compaction thread again. Perform the put and memtable flushes // until we see timeout after 6 memtable flushes. - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"level0_stop_writes_trigger", "6"} })); dbfull()->CompactRange(nullptr, nullptr); @@ -8703,7 +8703,7 @@ TEST(DBTest, DynamicCompactionOptions) { // 4 L0 files and compaction should be triggered. If auto compaction is // disabled, then TEST_WaitForCompact will be waiting for nothing. Number of // L0 files do not change after the call. - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"disable_auto_compactions", "true"} })); dbfull()->CompactRange(nullptr, nullptr); @@ -8719,7 +8719,7 @@ TEST(DBTest, DynamicCompactionOptions) { // Enable auto compaction and perform the same test, # of L0 files should be // reduced after compaction. - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"disable_auto_compactions", "false"} })); dbfull()->CompactRange(nullptr, nullptr); @@ -8737,7 +8737,7 @@ TEST(DBTest, DynamicCompactionOptions) { // First change max_bytes_for_level_base to a big value and populate // L1 - L3. Then thrink max_bytes_for_level_base and disable auto compaction // at the same time, we should see some level with score greater than 2. - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_bytes_for_level_base", std::to_string(k1MB) } })); // writing 40 x 64KB = 10 x 256KB @@ -8754,7 +8754,7 @@ TEST(DBTest, DynamicCompactionOptions) { SizeAtLevel(3) < 4 * k1MB * 1.2)); // Reduce max_bytes_for_level_base and disable compaction at the same time // This should cause score to increase - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"disable_auto_compactions", "true"}, {"max_bytes_for_level_base", "65536"}, })); @@ -8769,7 +8769,7 @@ TEST(DBTest, DynamicCompactionOptions) { // Enfoce hard rate limit. Now set hard_rate_limit to 2, // we should start to see put delay (1000 us) and timeout as a result // (L0 score is not regulated by this limit). - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"hard_rate_limit", "2"} })); ASSERT_OK(Put(Key(count), RandomString(&rnd, 1024))); @@ -8781,7 +8781,7 @@ TEST(DBTest, DynamicCompactionOptions) { ASSERT_TRUE(Put(Key(count), RandomString(&rnd, 1024), wo).IsTimedOut()); // Lift the limit and no timeout - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"hard_rate_limit", "100"} })); dbfull()->TEST_FlushMemTable(true); @@ -8807,7 +8807,7 @@ TEST(DBTest, DynamicCompactionOptions) { ASSERT_TRUE(Put("max_mem_compaction_level_key", RandomString(&rnd, 8)).ok()); // Set new value and it becomes effective in this flush - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_mem_compaction_level", "1"} })); dbfull()->TEST_FlushMemTable(true); @@ -8818,7 +8818,7 @@ TEST(DBTest, DynamicCompactionOptions) { ASSERT_TRUE(Put("max_mem_compaction_level_key", RandomString(&rnd, 8)).ok()); // Set new value and it becomes effective in this flush - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_mem_compaction_level", "0"} })); dbfull()->TEST_FlushMemTable(true); @@ -8994,7 +8994,7 @@ TEST(DBTest, DynamicMiscOptions) { // No reseek assert_reseek_count(100, 0); - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_sequential_skip_in_iterations", "4"} })); // Clear memtable and make new option effective @@ -9002,7 +9002,7 @@ TEST(DBTest, DynamicMiscOptions) { // Trigger reseek assert_reseek_count(200, 1); - ASSERT_TRUE(dbfull()->SetOptions({ + ASSERT_OK(dbfull()->SetOptions({ {"max_sequential_skip_in_iterations", "16"} })); // Clear memtable and make new option effective diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index 0653a8386..21fa43838 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -359,11 +359,11 @@ class DB { return CompactRange(DefaultColumnFamily(), begin, end, reduce_level, target_level, target_path_id); } - virtual bool SetOptions(ColumnFamilyHandle* column_family, + virtual Status SetOptions(ColumnFamilyHandle* column_family, const std::unordered_map& new_options) { - return true; + return Status::NotSupported("Not implemented"); } - virtual bool SetOptions( + virtual Status SetOptions( const std::unordered_map& new_options) { return SetOptions(DefaultColumnFamily(), new_options); } diff --git a/include/rocksdb/utilities/stackable_db.h b/include/rocksdb/utilities/stackable_db.h index 417378f5d..50c6a6484 100644 --- a/include/rocksdb/utilities/stackable_db.h +++ b/include/rocksdb/utilities/stackable_db.h @@ -3,6 +3,7 @@ // found in the LICENSE file. See the AUTHORS file for names of contributors. #pragma once +#include #include "rocksdb/db.h" namespace rocksdb { @@ -203,6 +204,12 @@ class StackableDB : public DB { return db_->GetDbIdentity(identity); } + using DB::SetOptions; + virtual Status SetOptions( + const std::unordered_map& new_options) override { + return db_->SetOptions(new_options); + } + using DB::GetPropertiesOfAllTables; virtual Status GetPropertiesOfAllTables(ColumnFamilyHandle* column_family, TablePropertiesCollection* props) { diff --git a/tools/db_stress.cc b/tools/db_stress.cc index d2bdec7e0..42d0fb534 100644 --- a/tools/db_stress.cc +++ b/tools/db_stress.cc @@ -1298,7 +1298,7 @@ class StressTest { return s; } - bool SetOptions(ThreadState* thread) { + Status SetOptions(ThreadState* thread) { assert(FLAGS_set_options_one_in > 0); std::unordered_map opts; std::string name = options_index_[ diff --git a/util/options_helper.cc b/util/options_helper.cc index 9b95150c5..268a67a99 100644 --- a/util/options_helper.cc +++ b/util/options_helper.cc @@ -165,7 +165,7 @@ bool ParseMiscOptions(const std::string& name, const std::string& value, return true; } -bool GetMutableOptionsFromStrings( +Status GetMutableOptionsFromStrings( const MutableCFOptions& base_options, const std::unordered_map& options_map, MutableCFOptions* new_options) { @@ -177,13 +177,14 @@ bool GetMutableOptionsFromStrings( } else if (ParseCompactionOptions(o.first, o.second, new_options)) { } else if (ParseMiscOptions(o.first, o.second, new_options)) { } else { - return false; + return Status::InvalidArgument( + "unsupported dynamic option: " + o.first); } } - } catch (std::exception) { - return false; + } catch (std::exception& e) { + return Status::InvalidArgument("error parsing " + std::string(e.what())); } - return true; + return Status::OK(); } namespace { diff --git a/util/options_helper.h b/util/options_helper.h index c04d2a5d7..62373b2d5 100644 --- a/util/options_helper.h +++ b/util/options_helper.h @@ -7,10 +7,11 @@ #include #include "util/mutable_cf_options.h" +#include "rocksdb/status.h" namespace rocksdb { -bool GetMutableOptionsFromStrings( +Status GetMutableOptionsFromStrings( const MutableCFOptions& base_options, const std::unordered_map& options_map, MutableCFOptions* new_options);