BugFix 3.3: adjust SetThrottle() to only call GetDelayToken() first time. (arangodb#5854)

matthewvon · neunhoef · commit 225095d7625c · 2018-07-12T20:01:41.000+02:00
* adjust SetThrottle() to only call GetDelayToken() first time.  Each call disrupts throttle timing.

* add ChangeLog entry
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,9 @@
 v3.3.12 (2018-07-12)
 --------------------
 
+* PR5854: rocksdb engine would frequently request a new DelayToken.  This caused
+  excessive write delay on the next Put() call.  Alternate approach taken.
+
 * fixed graph creation under some circumstances failing with 'edge collection
   already used in edge def' despite the edge definitions being identical
 
@@ -45,7 +48,7 @@ v3.3.11 (2018-06-26)
   more than a single shard and using a custom shard key (i.e. some shard
   key attribute other than `_key`).
   The previous implementation of DOCUMENT restricted to lookup to a single
-  shard in all cases, though this restriction was invalid. That lead to 
+  shard in all cases, though this restriction was invalid. That lead to
   `DOCUMENT` not finding documents in cases the wrong shard was contacted. The
   fixed implementation in 3.3.11 will reach out to all shards to find the
   document, meaning it will produce the correct result, but will cause more
@@ -59,7 +62,7 @@ v3.3.11 (2018-06-26)
 
 * fixed internal issue #2256: ui, document id not showing up when deleting a document
 
-* fixed internal issue #2163: wrong labels within foxx validation of service 
+* fixed internal issue #2163: wrong labels within foxx validation of service
   input parameters
 
 * fixed internal issue #2160: fixed misplaced tooltips in indices view
@@ -235,7 +238,7 @@ v3.3.9 (2018-05-17)
   `COLLECT ...` (no options)                 => create a plan using sorted, and another plan using hash method
 
 * added bulk document lookups for MMFiles engine, which will improve the performance
-  of document lookups from an inside an index in case the index lookup produces many 
+  of document lookups from an inside an index in case the index lookup produces many
   documents
 
 
diff --git a/arangod/RocksDBEngine/RocksDBThrottle.cpp b/arangod/RocksDBEngine/RocksDBThrottle.cpp
@@ -440,10 +440,24 @@ void RocksDBThrottle::SetThrottle() {
         // hard casting away of "const" ...
         if (((WriteController&)_internalRocksDB->write_controller()).max_delayed_write_rate() < _throttleBps) {
           ((WriteController&)_internalRocksDB->write_controller()).set_max_delayed_write_rate(_throttleBps);
-        } //if
-        _delayToken=(((WriteController&)_internalRocksDB->write_controller()).GetDelayToken(_throttleBps));
+        } // if
+
+        // Only replace the token when absolutely necessary.  GetDelayToken()
+        //  also resets internal timers which can result in long pauses if
+        //  flushes/compactions are happening often.
+        if (nullptr == _delayToken.get()) {
+          _delayToken=(((WriteController&)_internalRocksDB->write_controller()).GetDelayToken(_throttleBps));
+          LOG_TOPIC(DEBUG, arangodb::Logger::ENGINES)
+            << "SetThrottle(): GetDelayTokey(" << _throttleBps << ")";
+        } else {
+          LOG_TOPIC(DEBUG, arangodb::Logger::ENGINES)
+            << "SetThrottle(): set_delayed_write_rate(" << _throttleBps << ")";
+          ((WriteController&)_internalRocksDB->write_controller()).set_delayed_write_rate(_throttleBps);
+        } // else
       } else {
         _delayToken.reset();
+        LOG_TOPIC(DEBUG, arangodb::Logger::ENGINES)
+          << "SetThrottle(): _delaytoken.reset()";
       } // else
     } // if
   } // lock
