Dirty Memory: Revert allow_stall naming from allow_delays_and_stalls

HISTORY.md

@@ -7,9 +7,9 @@ Based on RocksDB 8.1.1
 * Delay writes gradually based on memory usage of the WriteBufferManager (WBM).
 Before this PR, setting allow_stall in the WBM's constructor meant that writes are completely stopped when the WBM's memory usage exceeds its quota. The goal here is to gradually decrease
 the users write speed before that threshold is reached in order to gain stability.
-To use this feature, pass allow_delays_and_stalls = true to the ctor of WBM (renamed from allow_stall) and the db needs to be opened with options.use_dynamic_delay = true. The WBM will
-setup delay requests starting from (start_delay_percent * _buffer_size) / 100 (default value is 70) (start_delay_percent is another WBM ctor parameter). Changes to the WBM's memory are tracked in WriteBufferManager::ReserveMem and FreeMem.
-Once the WBM reached its capacity, writes will be stopped using the old ShouldStall() and WBMStallWrites(). (#423)
+To use this feature, pass allow_stall = true to the ctor of WBM and the db needs to be opened with options.use_dynamic_delay = true. The WBM will setup delay requests starting from (start_delay_percent * _buffer_size) / 100 (default value is 70) (start_delay_percent is another WBM ctor parameter).
+Changes to the WBM's memory are tracked in WriteBufferManager::ReserveMem and FreeMem.
+Once the WBM reached its capacity, if allow_stall == true, writes will be stopped using the old ShouldStall() and WBMStallWrites(). (#423)
 
 * Prevent flush entry followed delete operations
 currently during memtable flush ,  if key has a match key in the

db/db_test.cc

@@ -5099,7 +5099,7 @@ TEST_F(DBTest, FlushOnDestroy) {
   CancelAllBackgroundWork(db_);
 }
 
-// stuck since allow_delays_and_stalls is now true which leads to ShouldStall()
+// stuck since allow_stall is now true which leads to ShouldStall()
 // to return true, but together with ShouldFlush() returning false since
 // initiate_flushes_ is true, there are no flushes. caused and will be fixed
 // with - https://github.com/speedb-io/speedb/issues/424

db/db_test2.cc

@@ -341,12 +341,12 @@ TEST_P(DBTestSharedWriteBufferAcrossCFs, SharedWriteBufferAcrossCFs) {
   if (use_old_interface_) {
     options.db_write_buffer_size = 120000;  // this is the real limit
   } else if (!cost_cache_) {
-    options.write_buffer_manager.reset(new WriteBufferManager(
-        114285, {}, WriteBufferManager::kDfltAllowDelaysAndStalls,
-        false /* initiate_flushes */));
+    options.write_buffer_manager.reset(
+        new WriteBufferManager(114285, {}, WriteBufferManager::kDfltAllowStall,
+                               false /* initiate_flushes */));
   } else {
     options.write_buffer_manager.reset(new WriteBufferManager(
-        114285, cache, WriteBufferManager::kDfltAllowDelaysAndStalls,
+        114285, cache, WriteBufferManager::kDfltAllowStall,
         false /* initiate_flushes */));
   }
   options.write_buffer_size = 500000;  // this is never hit
@@ -527,7 +527,7 @@ TEST_F(DBTest2, SharedWriteBufferLimitAcrossDB) {
   // Use a write buffer total size so that the soft limit is about
   // 105000.
   options.write_buffer_manager.reset(new WriteBufferManager(
-      120000, {} /* cache */, WriteBufferManager::kDfltAllowDelaysAndStalls,
+      120000, {} /* cache */, WriteBufferManager::kDfltAllowStall,
       false /* initiate_flushes */));
   CreateAndReopenWithCF({"cf1", "cf2"}, options);
 

db/global_write_controller_test.cc

@@ -42,8 +42,9 @@ class GlobalWriteControllerTest : public DBTestBase {
         options.use_dynamic_delay, options.delayed_write_rate));
     if (add_wbm) {
       options.write_buffer_manager.reset(new WriteBufferManager(
-          buffer_size, {}, true /*allow_delays_and_stalls*/,
-          false /*initiate_flushes*/));
+          buffer_size, {}, true /*allow_stall*/, false /*initiate_flushes*/,
+          WriteBufferManager::FlushInitiationOptions(),
+          WriteBufferManager::kDfltStartDelayPercentThreshold));
     }
 
     for (int i = 0; i < num_dbs; i++) {

db_stress_tool/db_stress_common.h

@@ -104,7 +104,7 @@ DECLARE_bool(verbose);
 DECLARE_bool(progress_reports);
 DECLARE_uint64(db_write_buffer_size);
 DECLARE_bool(cost_write_buffer_to_cache);
-DECLARE_bool(allow_wbm_delays_and_stalls);
+DECLARE_bool(allow_wbm_stalls);
 DECLARE_uint32(start_delay_percent);
 DECLARE_bool(initiate_wbm_flushes);
 DECLARE_uint32(max_num_parallel_flushes);

db_stress_tool/db_stress_gflags.cc

@@ -140,8 +140,8 @@ DEFINE_uint64(db_write_buffer_size,
 DEFINE_bool(cost_write_buffer_to_cache, false,
             "The usage of memtable is costed to the block cache");
 
-DEFINE_bool(allow_wbm_delays_and_stalls,
-            ROCKSDB_NAMESPACE::WriteBufferManager::kDfltAllowDelaysAndStalls,
+DEFINE_bool(allow_wbm_stalls,
+            ROCKSDB_NAMESPACE::WriteBufferManager::kDfltAllowStall,
             "Enable WBM write stalls and delays");
 
 DEFINE_uint32(

db_stress_tool/db_stress_test_base.cc

@@ -2400,8 +2400,8 @@ void StressTest::PrintEnv() const {
           FLAGS_db_write_buffer_size);
   fprintf(stdout, "Cost To Cache (WBM)       : %s\n",
           FLAGS_cost_write_buffer_to_cache ? "true" : "false");
-  fprintf(stdout, "Allow WBM Delays and Stalls: %s\n",
-          FLAGS_allow_wbm_delays_and_stalls ? "true" : "false");
+  fprintf(stdout, "Allow WBM Stalls and Delays: %s\n",
+          FLAGS_allow_wbm_stalls ? "true" : "false");
   fprintf(stdout, "WBM start delay percent   : %d\n",
           FLAGS_start_delay_percent);
   fprintf(stdout, "Initiate WBM Flushes      : %s\n",
@@ -3108,13 +3108,13 @@ void InitializeOptionsFromFlags(
   }
   if (FLAGS_cost_write_buffer_to_cache) {
     options.write_buffer_manager.reset(new WriteBufferManager(
-        FLAGS_db_write_buffer_size, cache, FLAGS_allow_wbm_delays_and_stalls,
-        FLAGS_initiate_wbm_flushes, flush_initiation_options));
+        FLAGS_db_write_buffer_size, cache, FLAGS_allow_wbm_stalls,
+        FLAGS_initiate_wbm_flushes, flush_initiation_options,
+        static_cast<uint16_t>(FLAGS_start_delay_percent)));
   } else {
     options.write_buffer_manager.reset(new WriteBufferManager(
-        FLAGS_db_write_buffer_size, {} /* cache */,
-        FLAGS_allow_wbm_delays_and_stalls, FLAGS_initiate_wbm_flushes,
-        flush_initiation_options,
+        FLAGS_db_write_buffer_size, {} /* cache */, FLAGS_allow_wbm_stalls,
+        FLAGS_initiate_wbm_flushes, flush_initiation_options,
         static_cast<uint16_t>(FLAGS_start_delay_percent)));
   }
 

include/rocksdb/write_buffer_manager.h

@@ -46,7 +46,7 @@ class StallInterface {
 
 class WriteBufferManager final {
  public:
-  // Delay Mechanism (allow_delays_and_stalls == true) definitions
+  // Delay Mechanism (allow_stall == true) definitions
   static constexpr uint16_t kDfltStartDelayPercentThreshold = 70U;
   static constexpr uint64_t kNoDelayedWriteFactor = 0U;
   static constexpr uint64_t kMaxDelayedWriteFactor = 100U;
@@ -72,7 +72,7 @@ class WriteBufferManager final {
     size_t max_num_parallel_flushes = kDfltMaxNumParallelFlushes;
   };
 
-  static constexpr bool kDfltAllowDelaysAndStalls = true;
+  static constexpr bool kDfltAllowStall = false;
   static constexpr bool kDfltInitiateFlushes = true;
 
  public:
@@ -84,7 +84,7 @@ class WriteBufferManager final {
   // cost the memory allocated to the cache. It can be used even if _buffer_size
   // = 0.
   //
-  // allow_delays_and_stalls: if set true, will enable delays and stall as
+  // allow_stall: if set true, will enable delays and stall as
   // described below:
   //  Delays: delay writes when memory_usage() exceeds the
   //    start_delay_percent percent threshold of the buffer size.
@@ -102,7 +102,7 @@ class WriteBufferManager final {
   // write-path of a DB.
   explicit WriteBufferManager(
       size_t _buffer_size, std::shared_ptr<Cache> cache = {},
-      bool allow_delays_and_stalls = kDfltAllowDelaysAndStalls,
+      bool allow_stall = kDfltAllowStall,
       bool initiate_flushes = kDfltInitiateFlushes,
       const FlushInitiationOptions& flush_initiation_options =
           FlushInitiationOptions(),
@@ -201,12 +201,11 @@ class WriteBufferManager final {
   // We stall the writes untill memory_usage drops below buffer_size. When the
   // function returns true, all writer threads (including one checking this
   // condition) across all DBs will be stalled. Stall is allowed only if user
-  // pass allow_delays_and_stalls = true during WriteBufferManager instance
-  // creation.
+  // pass allow_stall = true during WriteBufferManager instance creation.
   //
   // Should only be called by RocksDB internally .
   bool ShouldStall() const {
-    if (!allow_delays_and_stalls_ || !enabled()) {
+    if (!allow_stall_ || !enabled()) {
       return false;
     }
 
@@ -354,7 +353,7 @@ class WriteBufferManager final {
   std::list<StallInterface*> queue_;
   // Protects the queue_ and stall_active_.
   std::mutex mu_;
-  bool allow_delays_and_stalls_ = kDfltAllowDelaysAndStalls;
+  bool allow_stall_ = kDfltAllowStall;
   uint16_t start_delay_percent_ = kDfltStartDelayPercentThreshold;
 
   // Value should only be changed by BeginWriteStall() and MaybeEndWriteStall()

memtable/write_buffer_manager.cc

@@ -33,8 +33,8 @@ auto WriteBufferManager::FlushInitiationOptions::Sanitize() const
 }
 
 WriteBufferManager::WriteBufferManager(
-    size_t _buffer_size, std::shared_ptr<Cache> cache,
-    bool allow_delays_and_stalls, bool initiate_flushes,
+    size_t _buffer_size, std::shared_ptr<Cache> cache, bool allow_stall,
+    bool initiate_flushes,
     const FlushInitiationOptions& flush_initiation_options,
     uint16_t start_delay_percent)
     : buffer_size_(_buffer_size),
@@ -43,7 +43,7 @@ WriteBufferManager::WriteBufferManager(
       memory_inactive_(0),
       memory_being_freed_(0U),
       cache_res_mgr_(nullptr),
-      allow_delays_and_stalls_(allow_delays_and_stalls),
+      allow_stall_(allow_stall),
       start_delay_percent_(start_delay_percent),
       stall_active_(false),
       initiate_flushes_(initiate_flushes),
@@ -210,7 +210,7 @@ size_t WriteBufferManager::FreeMemWithCache(size_t mem) {
 
 void WriteBufferManager::BeginWriteStall(StallInterface* wbm_stall) {
   assert(wbm_stall != nullptr);
-  assert(allow_delays_and_stalls_);
+  assert(allow_stall_);
 
   // Allocate outside of the lock.
   std::list<StallInterface*> new_node = {wbm_stall};
@@ -235,7 +235,7 @@ void WriteBufferManager::BeginWriteStall(StallInterface* wbm_stall) {
 void WriteBufferManager::MaybeEndWriteStall() {
   // Cannot early-exit on !enabled() because SetBufferSize(0) needs to unblock
   // the writers.
-  if (!allow_delays_and_stalls_) {
+  if (!allow_stall_) {
     return;
   }
 
@@ -267,7 +267,7 @@ void WriteBufferManager::RemoveDBFromQueue(StallInterface* wbm_stall) {
   // Deallocate the removed nodes outside of the lock.
   std::list<StallInterface*> cleanup;
 
-  if (enabled() && allow_delays_and_stalls_) {
+  if (enabled() && allow_stall_) {
     std::unique_lock<std::mutex> lock(mu_);
     for (auto it = queue_.begin(); it != queue_.end();) {
       auto next = std::next(it);
@@ -302,16 +302,16 @@ std::string WriteBufferManager::GetPrintableOptions() const {
   snprintf(buffer, kBufferSize, "%*s: %p\n", field_width, "wbm.cache", cache);
   ret.append(buffer);
 
-  snprintf(buffer, kBufferSize, "%*s: %d\n", field_width,
-           "wbm.allow_delays_and_stalls", allow_delays_and_stalls_);
+  snprintf(buffer, kBufferSize, "%*s: %d\n", field_width, "wbm.allow_stall",
+           allow_stall_);
   ret.append(buffer);
 
   snprintf(buffer, kBufferSize, "%*s: %d\n", field_width,
-           "wbm.initiate_flushes", IsInitiatingFlushes());
+           "wbm.start_delay_percent", start_delay_percent_);
   ret.append(buffer);
 
   snprintf(buffer, kBufferSize, "%*s: %d\n", field_width,
-           "wbm.start_delay_percent", start_delay_percent_);
+           "wbm.initiate_flushes", IsInitiatingFlushes());
   ret.append(buffer);
 
   return ret;
@@ -520,8 +520,7 @@ void WriteBufferManager::UpdateUsageState(size_t new_memory_used,
                                           ssize_t memory_changed_size,
                                           size_t quota) {
   assert(enabled());
-  if (allow_delays_and_stalls_ == false ||
-      controllers_to_refcount_map_.empty()) {
+  if (allow_stall_ == false) {
     return;
   }
 

memtable/write_buffer_manager_test.cc

@@ -40,10 +40,9 @@ void ScheduleBeginAndFreeMem(WriteBufferManager& wbf, size_t size) {
 
 TEST_F(WriteBufferManagerTest, ShouldFlush) {
   // A write buffer manager of size 10MB
-  std::unique_ptr<WriteBufferManager> wbf(
-      new WriteBufferManager(10 * 1024 * 1024, {} /* cache */,
-                             WriteBufferManager::kDfltAllowDelaysAndStalls,
-                             false /* initiate_flushes */));
+  std::unique_ptr<WriteBufferManager> wbf(new WriteBufferManager(
+      10 * 1024 * 1024, {} /* cache */, WriteBufferManager::kDfltAllowStall,
+      false /* initiate_flushes */));
 
   wbf->ReserveMem(8 * 1024 * 1024);
   ASSERT_FALSE(wbf->ShouldFlush());
@@ -114,7 +113,7 @@ TEST_F(ChargeWriteBufferTest, Basic) {
   std::shared_ptr<Cache> cache = NewLRUCache(co);
   // A write buffer manager of size 50MB
   std::unique_ptr<WriteBufferManager> wbf(new WriteBufferManager(
-      50 * 1024 * 1024, cache, WriteBufferManager::kDfltAllowDelaysAndStalls,
+      50 * 1024 * 1024, cache, WriteBufferManager::kDfltAllowStall,
       false /* initiate_flushes */));
 
   // Allocate 333KB will allocate 512KB, memory_used_ = 333KB

tools/db_bench_tool.cc

@@ -541,8 +541,8 @@ DEFINE_int64(db_write_buffer_size,
 DEFINE_bool(cost_write_buffer_to_cache, false,
             "The usage of memtable is costed to the block cache");
 
-DEFINE_bool(allow_wbm_delays_and_stalls,
-            ROCKSDB_NAMESPACE::WriteBufferManager::kDfltAllowDelaysAndStalls,
+DEFINE_bool(allow_wbm_stalls,
+            ROCKSDB_NAMESPACE::WriteBufferManager::kDfltAllowStall,
             "Enable WBM write stalls and delays");
 
 DEFINE_bool(initiate_wbm_flushes,
@@ -4945,15 +4945,13 @@ class Benchmark {
     if (options.write_buffer_manager == nullptr) {
       if (FLAGS_cost_write_buffer_to_cache) {
         options.write_buffer_manager.reset(new WriteBufferManager(
-            FLAGS_db_write_buffer_size, cache_,
-            FLAGS_allow_wbm_delays_and_stalls, FLAGS_initiate_wbm_flushes,
-            flush_initiation_options,
+            FLAGS_db_write_buffer_size, cache_, FLAGS_allow_wbm_stalls,
+            FLAGS_initiate_wbm_flushes, flush_initiation_options,
             static_cast<uint16_t>(FLAGS_start_delay_percent)));
       } else {
         options.write_buffer_manager.reset(new WriteBufferManager(
-            FLAGS_db_write_buffer_size, {} /* cache */,
-            FLAGS_allow_wbm_delays_and_stalls, FLAGS_initiate_wbm_flushes,
-            flush_initiation_options,
+            FLAGS_db_write_buffer_size, {} /* cache */, FLAGS_allow_wbm_stalls,
+            FLAGS_initiate_wbm_flushes, flush_initiation_options,
             static_cast<uint16_t>(FLAGS_start_delay_percent)));
       }
     }

tools/db_crashtest.py

@@ -224,7 +224,7 @@
     # "filter_uri": lambda: random.choice(["speedb.PairedBloomFilter", ""]),
     "memtablerep": lambda: random.choice(["skip_list", "speedb.HashSpdRepFactory"]),
     "use_dynamic_delay": lambda: random.choice([0, 1, 1, 1]),
-    "allow_wbm_delays_and_stalls": lambda: random.randint(0, 1),
+    "allow_wbm_stalls": lambda: random.randint(0, 1),
     "start_delay_percent": lambda: random.randint(0, 99),
     "use_clean_delete_during_flush": lambda: random.randint(0, 1),
 }