Revert #252 and disable ignore_unknown_options only for lower version (…

…#258)

* Revert "Add an option to disable write stall (#251) (#252)"

This reverts commit 32b29a1.

Signed-off-by: tabokie <xy.tao@outlook.com>

* use ignore_unknown_options regardless of RocksDB version

Signed-off-by: tabokie <xy.tao@outlook.com>

* disable only for lower version and fix test

Signed-off-by: tabokie <xy.tao@outlook.com>

db/column_family.cc

@@ -738,7 +738,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions(
     bool needed_delay = write_controller->NeedsDelay();
 
     if (write_stall_condition == WriteStallCondition::kStopped &&
-        write_stall_cause == WriteStallCause::kMemtableLimit && !mutable_cf_options.disable_write_stall) {
+        write_stall_cause == WriteStallCause::kMemtableLimit) {
       write_controller_token_ = write_controller->GetStopToken();
       internal_stats_->AddCFStats(InternalStats::MEMTABLE_LIMIT_STOPS, 1);
       ROCKS_LOG_WARN(
@@ -748,7 +748,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions(
           name_.c_str(), imm()->NumNotFlushed(),
           mutable_cf_options.max_write_buffer_number);
     } else if (write_stall_condition == WriteStallCondition::kStopped &&
-               write_stall_cause == WriteStallCause::kL0FileCountLimit && !mutable_cf_options.disable_write_stall) {
+               write_stall_cause == WriteStallCause::kL0FileCountLimit) {
       write_controller_token_ = write_controller->GetStopToken();
       internal_stats_->AddCFStats(InternalStats::L0_FILE_COUNT_LIMIT_STOPS, 1);
       if (compaction_picker_->IsLevel0CompactionInProgress()) {
@@ -759,7 +759,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions(
                      "[%s] Stopping writes because we have %d level-0 files",
                      name_.c_str(), vstorage->l0_delay_trigger_count());
     } else if (write_stall_condition == WriteStallCondition::kStopped &&
-               write_stall_cause == WriteStallCause::kPendingCompactionBytes && !mutable_cf_options.disable_write_stall) {
+               write_stall_cause == WriteStallCause::kPendingCompactionBytes) {
       write_controller_token_ = write_controller->GetStopToken();
       internal_stats_->AddCFStats(
           InternalStats::PENDING_COMPACTION_BYTES_LIMIT_STOPS, 1);
@@ -769,7 +769,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions(
           "bytes %" PRIu64,
           name_.c_str(), compaction_needed_bytes);
     } else if (write_stall_condition == WriteStallCondition::kDelayed &&
-               write_stall_cause == WriteStallCause::kMemtableLimit && !mutable_cf_options.disable_write_stall) {
+               write_stall_cause == WriteStallCause::kMemtableLimit) {
       write_controller_token_ =
           SetupDelay(write_controller, compaction_needed_bytes,
                      prev_compaction_needed_bytes_, was_stopped,
@@ -784,7 +784,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions(
           mutable_cf_options.max_write_buffer_number,
           write_controller->delayed_write_rate());
     } else if (write_stall_condition == WriteStallCondition::kDelayed &&
-               write_stall_cause == WriteStallCause::kL0FileCountLimit && !mutable_cf_options.disable_write_stall) {
+               write_stall_cause == WriteStallCause::kL0FileCountLimit) {
       // L0 is the last two files from stopping.
       bool near_stop = vstorage->l0_delay_trigger_count() >=
                        mutable_cf_options.level0_stop_writes_trigger - 2;
@@ -804,7 +804,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions(
                      name_.c_str(), vstorage->l0_delay_trigger_count(),
                      write_controller->delayed_write_rate());
     } else if (write_stall_condition == WriteStallCondition::kDelayed &&
-               write_stall_cause == WriteStallCause::kPendingCompactionBytes && !mutable_cf_options.disable_write_stall) {
+               write_stall_cause == WriteStallCause::kPendingCompactionBytes) {
       // If the distance to hard limit is less than 1/4 of the gap between soft
       // and
       // hard bytes limit, we think it is near stop and speed up the slowdown.
@@ -829,7 +829,7 @@ WriteStallCondition ColumnFamilyData::RecalculateWriteStallConditions(
           name_.c_str(), vstorage->estimated_compaction_needed_bytes(),
           write_controller->delayed_write_rate());
     } else {
-      assert(write_stall_condition == WriteStallCondition::kNormal || mutable_cf_options.disable_write_stall);
+      assert(write_stall_condition == WriteStallCondition::kNormal);
       if (vstorage->l0_delay_trigger_count() >=
           GetL0ThresholdSpeedupCompaction(
               mutable_cf_options.level0_file_num_compaction_trigger,

db/db_impl/db_impl.cc

@@ -993,13 +993,11 @@ Status DBImpl::SetDBOptions(
           GetBGJobLimits(mutable_db_options_.max_background_flushes,
                          mutable_db_options_.max_background_compactions,
                          mutable_db_options_.max_background_jobs,
-                         mutable_db_options_.base_background_compactions,
                          /* parallelize_compactions */ true);
       const BGJobLimits new_bg_job_limits = GetBGJobLimits(
           new_options.max_background_flushes,
           new_options.max_background_compactions,
-          new_options.max_background_jobs,
-          new_options.base_background_compactions, /* parallelize_compactions */ true);
+          new_options.max_background_jobs, /* parallelize_compactions */ true);
 
       const bool max_flushes_increased =
           new_bg_job_limits.max_flushes > current_bg_job_limits.max_flushes;

db/db_impl/db_impl.h

@@ -528,7 +528,6 @@ class DBImpl : public DB {
   static BGJobLimits GetBGJobLimits(int max_background_flushes,
                                     int max_background_compactions,
                                     int max_background_jobs,
-                                    int base_background_compactions,
                                     bool parallelize_compactions);
 
   // move logs pending closing from job_context to the DB queue and

db/db_impl/db_impl_compaction_flush.cc

@@ -1965,14 +1965,12 @@ DBImpl::BGJobLimits DBImpl::GetBGJobLimits() const {
   return GetBGJobLimits(mutable_db_options_.max_background_flushes,
                         mutable_db_options_.max_background_compactions,
                         mutable_db_options_.max_background_jobs,
-                        mutable_db_options_.base_background_compactions,
                         write_controller_.NeedSpeedupCompaction());
 }
 
 DBImpl::BGJobLimits DBImpl::GetBGJobLimits(int max_background_flushes,
                                            int max_background_compactions,
                                            int max_background_jobs,
-                                           int base_background_compactions,
                                            bool parallelize_compactions) {
   BGJobLimits res;
   if (max_background_flushes == -1 && max_background_compactions == -1) {
@@ -1988,7 +1986,7 @@ DBImpl::BGJobLimits DBImpl::GetBGJobLimits(int max_background_flushes,
   }
   if (!parallelize_compactions) {
     // throttle background compactions until we deem necessary
-    res.max_compactions = std::max(1, std::min(base_background_compactions, res.max_compactions));
+    res.max_compactions = 1;
   }
   return res;
 }

db/db_impl/db_impl_open.cc

@@ -57,7 +57,7 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src) {
   }
   auto bg_job_limits = DBImpl::GetBGJobLimits(
       result.max_background_flushes, result.max_background_compactions,
-      result.max_background_jobs, result.base_background_compactions, true /* parallelize_compactions */);
+      result.max_background_jobs, true /* parallelize_compactions */);
   result.env->IncBackgroundThreadsIfNeeded(bg_job_limits.max_compactions,
                                            Env::Priority::LOW);
   result.env->IncBackgroundThreadsIfNeeded(bg_job_limits.max_flushes,

db/db_options_test.cc

@@ -376,88 +376,6 @@ TEST_F(DBOptionsTest, EnableAutoCompactionAndTriggerStall) {
   }
 }
 
-TEST_F(DBOptionsTest, EnableAutoCompactionButDisableStall) {
-  const std::string kValue(1024, 'v');
-  Options options;
-  options.create_if_missing = true;
-  options.disable_auto_compactions = true;
-  options.disable_write_stall = true;
-  options.write_buffer_size = 1024 * 1024 * 10;
-  options.compression = CompressionType::kNoCompression;
-  options.level0_file_num_compaction_trigger = 1;
-  options.level0_stop_writes_trigger = std::numeric_limits<int>::max();
-  options.level0_slowdown_writes_trigger = std::numeric_limits<int>::max();
-  options.hard_pending_compaction_bytes_limit =
-      std::numeric_limits<uint64_t>::max();
-  options.soft_pending_compaction_bytes_limit =
-      std::numeric_limits<uint64_t>::max();
-  options.env = env_;
-
-  DestroyAndReopen(options);
-  int i = 0;
-  for (; i < 1024; i++) {
-    Put(Key(i), kValue);
-  }
-  Flush();
-  for (; i < 1024 * 2; i++) {
-    Put(Key(i), kValue);
-  }
-  Flush();
-  dbfull()->TEST_WaitForFlushMemTable();
-  ASSERT_EQ(2, NumTableFilesAtLevel(0));
-  uint64_t l0_size = SizeAtLevel(0);
-
-  options.hard_pending_compaction_bytes_limit = l0_size;
-  options.soft_pending_compaction_bytes_limit = l0_size;
-
-  Reopen(options);
-  dbfull()->TEST_WaitForCompact();
-  ASSERT_FALSE(dbfull()->TEST_write_controler().IsStopped());
-  ASSERT_FALSE(dbfull()->TEST_write_controler().NeedsDelay());
-
-  SyncPoint::GetInstance()->LoadDependency(
-      {{"DBOptionsTest::EnableAutoCompactionButDisableStall:1",
-        "BackgroundCallCompaction:0"},
-        {"DBImpl::BackgroundCompaction():BeforePickCompaction",
-        "DBOptionsTest::EnableAutoCompactionButDisableStall:2"},
-        {"DBOptionsTest::EnableAutoCompactionButDisableStall:3",
-        "DBImpl::BackgroundCompaction():AfterPickCompaction"}});
-  // Block background compaction.
-  SyncPoint::GetInstance()->EnableProcessing();
-
-  ASSERT_OK(
-      dbfull()->SetOptions({{"disable_auto_compactions", "false"}}));
-  TEST_SYNC_POINT("DBOptionsTest::EnableAutoCompactionButDisableStall:1");
-  // Wait for stall condition recalculate.
-  TEST_SYNC_POINT("DBOptionsTest::EnableAutoCompactionButDisableStall:2");
-
-  ASSERT_FALSE(dbfull()->TEST_write_controler().IsStopped());
-  ASSERT_FALSE(dbfull()->TEST_write_controler().NeedsDelay());
-  ASSERT_TRUE(dbfull()->TEST_write_controler().NeedSpeedupCompaction());
-
-  TEST_SYNC_POINT("DBOptionsTest::EnableAutoCompactionButDisableStall:3");
-
-  // Background compaction executed.
-  dbfull()->TEST_WaitForCompact();
-  ASSERT_FALSE(dbfull()->TEST_write_controler().IsStopped());
-  ASSERT_FALSE(dbfull()->TEST_write_controler().NeedsDelay());
-  ASSERT_FALSE(dbfull()->TEST_write_controler().NeedSpeedupCompaction());
-}
-
-TEST_F(DBOptionsTest, SetOptionsDisableWriteStall) {
-  Options options;
-  options.create_if_missing = true;
-  options.disable_write_stall = false;
-  options.env = env_;
-  Reopen(options);
-  ASSERT_EQ(false, dbfull()->GetOptions().disable_write_stall);
-
-  ASSERT_OK(dbfull()->SetOptions({{"disable_write_stall", "true"}}));
-  ASSERT_EQ(true, dbfull()->GetOptions().disable_write_stall);
-  ASSERT_OK(dbfull()->SetOptions({{"disable_write_stall", "false"}}));
-  ASSERT_EQ(false, dbfull()->GetOptions().disable_write_stall);
-}
-
 TEST_F(DBOptionsTest, SetOptionsMayTriggerCompaction) {
   Options options;
   options.create_if_missing = true;
@@ -486,21 +404,8 @@ TEST_F(DBOptionsTest, SetBackgroundCompactionThreads) {
   ASSERT_EQ(1, dbfull()->TEST_BGCompactionsAllowed());
   ASSERT_OK(dbfull()->SetDBOptions({{"max_background_compactions", "3"}}));
   ASSERT_EQ(1, dbfull()->TEST_BGCompactionsAllowed());
-  {
-    auto stop_token = dbfull()->TEST_write_controler().GetStopToken();
-    ASSERT_EQ(3, dbfull()->TEST_BGCompactionsAllowed());
-  }
-
-  ASSERT_OK(dbfull()->SetDBOptions({{"base_background_compactions", "2"}}));
-  ASSERT_EQ(2, dbfull()->TEST_BGCompactionsAllowed());
-  ASSERT_OK(dbfull()->SetDBOptions({{"base_background_compactions", "5"}}));
+  auto stop_token = dbfull()->TEST_write_controler().GetStopToken();
   ASSERT_EQ(3, dbfull()->TEST_BGCompactionsAllowed());
-  ASSERT_OK(dbfull()->SetDBOptions({{"base_background_compactions", "1"}}));
-  ASSERT_EQ(1, dbfull()->TEST_BGCompactionsAllowed());
-  {
-    auto stop_token = dbfull()->TEST_write_controler().GetStopToken();
-    ASSERT_EQ(3, dbfull()->TEST_BGCompactionsAllowed());
-  }
 }
 
 TEST_F(DBOptionsTest, SetBackgroundFlushThreads) {

include/rocksdb/options.h

@@ -290,11 +290,6 @@ struct ColumnFamilyOptions : public AdvancedColumnFamilyOptions {
   // Dynamically changeable through SetOptions() API
   bool disable_auto_compactions = false;
 
-  // Disable write stall mechanism.
-  //
-  // Dynamically changeable through SetOptions() API
-  bool disable_write_stall = false;
-
   // This is a factory that provides TableFactory objects.
   // Default: a block-based table factory that provides a default
   // implementation of TableBuilder and TableReader with default

options/cf_options.cc

@@ -148,8 +148,6 @@ void MutableCFOptions::Dump(Logger* log) const {
       prefix_extractor == nullptr ? "nullptr" : prefix_extractor->Name());
   ROCKS_LOG_INFO(log, "                 disable_auto_compactions: %d",
                  disable_auto_compactions);
-  ROCKS_LOG_INFO(log, "                      disable_write_stall: %d",
-                 disable_write_stall);
   ROCKS_LOG_INFO(log, "      soft_pending_compaction_bytes_limit: %" PRIu64,
                  soft_pending_compaction_bytes_limit);
   ROCKS_LOG_INFO(log, "      hard_pending_compaction_bytes_limit: %" PRIu64,

options/cf_options.h

@@ -139,7 +139,6 @@ struct MutableCFOptions {
         inplace_update_num_locks(options.inplace_update_num_locks),
         prefix_extractor(options.prefix_extractor),
         disable_auto_compactions(options.disable_auto_compactions),
-        disable_write_stall(options.disable_write_stall),
         soft_pending_compaction_bytes_limit(
             options.soft_pending_compaction_bytes_limit),
         hard_pending_compaction_bytes_limit(
@@ -180,7 +179,6 @@ struct MutableCFOptions {
         inplace_update_num_locks(0),
         prefix_extractor(nullptr),
         disable_auto_compactions(false),
-        disable_write_stall(false),
         soft_pending_compaction_bytes_limit(0),
         hard_pending_compaction_bytes_limit(0),
         level0_file_num_compaction_trigger(0),
@@ -233,7 +231,6 @@ struct MutableCFOptions {
 
   // Compaction related options
   bool disable_auto_compactions;
-  bool disable_write_stall;
   uint64_t soft_pending_compaction_bytes_limit;
   uint64_t hard_pending_compaction_bytes_limit;
   int level0_file_num_compaction_trigger;

options/db_options.cc

@@ -274,8 +274,6 @@ MutableDBOptions::MutableDBOptions(const DBOptions& options)
 void MutableDBOptions::Dump(Logger* log) const {
   ROCKS_LOG_HEADER(log, "            Options.max_background_jobs: %d",
                    max_background_jobs);
-  ROCKS_LOG_HEADER(log, "            Options.base_background_compactions: %d",
-                   base_background_compactions);
   ROCKS_LOG_HEADER(log, "            Options.max_background_compactions: %d",
                    max_background_compactions);
   ROCKS_LOG_HEADER(log, "            Options.avoid_flush_during_shutdown: %d",

options/options.cc

@@ -249,8 +249,6 @@ void ColumnFamilyOptions::Dump(Logger* log) const {
                      rate_limit_delay_max_milliseconds);
     ROCKS_LOG_HEADER(log, "               Options.disable_auto_compactions: %d",
                      disable_auto_compactions);
-    ROCKS_LOG_HEADER(log, "               Options.disable_write_stall: %d",
-                     disable_write_stall);
 
     const auto& it_compaction_style =
         compaction_style_to_string.find(compaction_style);

options/options_helper.cc

@@ -165,8 +165,6 @@ ColumnFamilyOptions BuildColumnFamilyOptions(
   // Compaction related options
   cf_opts.disable_auto_compactions =
       mutable_cf_options.disable_auto_compactions;
-  cf_opts.disable_write_stall = 
-      mutable_cf_options.disable_write_stall;
   cf_opts.soft_pending_compaction_bytes_limit =
       mutable_cf_options.soft_pending_compaction_bytes_limit;
   cf_opts.hard_pending_compaction_bytes_limit =
@@ -1809,10 +1807,6 @@ std::unordered_map<std::string, OptionTypeInfo>
          {offset_of(&ColumnFamilyOptions::disable_auto_compactions),
           OptionType::kBoolean, OptionVerificationType::kNormal, true,
           offsetof(struct MutableCFOptions, disable_auto_compactions)}},
-        {"disable_write_stall",
-         {offset_of(&ColumnFamilyOptions::disable_write_stall),
-          OptionType::kBoolean, OptionVerificationType::kNormal, true,
-          offsetof(struct MutableCFOptions, disable_write_stall)}},
         {"filter_deletes",
          {0, OptionType::kBoolean, OptionVerificationType::kDeprecated, true,
           0}},

options/options_parser.cc

@@ -234,11 +234,11 @@ Status RocksDBOptionsParser::Parse(const std::string& file_name, Env* env,
         return s;
       }
 
-      // If the option file is not generated by a higher minor version,
+      // If the option file is generated by a lower minor version,
       // there shouldn't be any unknown option.
       if (ignore_unknown_options && section == kOptionSectionVersion) {
-        if (db_version[0] < ROCKSDB_MAJOR || (db_version[0] == ROCKSDB_MAJOR &&
-                                              db_version[1] <= ROCKSDB_MINOR)) {
+        if (db_version[0] < ROCKSDB_MAJOR ||
+            (db_version[0] == ROCKSDB_MAJOR && db_version[1] < ROCKSDB_MINOR)) {
           ignore_unknown_options = false;
         }
       }

options/options_settable_test.cc

@@ -456,7 +456,6 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) {
       "compaction_pri=kMinOverlappingRatio;"
       "hard_pending_compaction_bytes_limit=0;"
       "disable_auto_compactions=false;"
-      "disable_write_stall=false;"
       "report_bg_io_stats=true;"
       "ttl=60;"
       "periodic_compaction_seconds=3600;"

options/options_test.cc

@@ -1307,7 +1307,7 @@ TEST_F(OptionsParserTest, IgnoreUnknownOptions) {
     bool should_ignore = true;
     if (case_id == 0) {
       // same version
-      should_ignore = false;
+      should_ignore = true;
       version_string =
           ToString(ROCKSDB_MAJOR) + "." + ToString(ROCKSDB_MINOR) + ".0";
     } else if (case_id == 1) {