Fix `GenericRateLimiter` hanging bug (#11763)

Summary:
Fixes https://github.com/facebook/rocksdb/issues/11742

Even after performing duty (1) ("Waiting for the next refill time"), it is possible the remaining threads are all in `Wait()`. Waking up at least one thread is enough to ensure progress continues, even if no new requests arrive.

The repro unit test (https://github.com/facebook/rocksdb/commit/bb54245e6) is not included as it depends on an unlanded PR (https://github.com/facebook/rocksdb/issues/11753)

Pull Request resolved: https://github.com/facebook/rocksdb/pull/11763

Reviewed By: jaykorean

Differential Revision: D48710130

Pulled By: ajkr

fbshipit-source-id: 9d166bd577ea3a96ccd81dde85871fec5e85a4eb

diff --git a/unreleased_history/bug_fixes/fixed_generic_rate_limiter_hang.md b/unreleased_history/bug_fixes/fixed_generic_rate_limiter_hang.md
new file mode 100644 (file)
index 0000000..8f789e1
--- /dev/null
+++ b/unreleased_history/bug_fixes/fixed_generic_rate_limiter_hang.md
@@ -0,0 +1 @@
+Fixed a race condition in `GenericRateLimiter` that could cause it to stop granting requests

diff --git a/util/rate_limiter.cc b/util/rate_limiter.cc
index be54138d94828884a9ca35bd169f378fba978f27..ddb9bdbf020b349d8d754b0f14d2e6e8c992b56d 100644 (file)
--- a/util/rate_limiter.cc
+++ b/util/rate_limiter.cc
@@ -179,16 +179,16 @@ void GenericRateLimiter::Request(int64_t bytes, const Env::IOPriority pri,
       // Whichever thread reaches here first performs duty (2) as described
       // above.
       RefillBytesAndGrantRequestsLocked();
-      if (r.request_bytes == 0) {
-        // If there is any remaining requests, make sure there exists at least
-        // one candidate is awake for future duties by signaling a front request
-        // of a queue.
-        for (int i = Env::IO_TOTAL - 1; i >= Env::IO_LOW; --i) {
-          std::deque<Req*> queue = queue_[i];
-          if (!queue.empty()) {
-            queue.front()->cv.Signal();
-            break;
-          }
+    }
+    if (r.request_bytes == 0) {
+      // If there is any remaining requests, make sure there exists at least
+      // one candidate is awake for future duties by signaling a front request
+      // of a queue.
+      for (int i = Env::IO_TOTAL - 1; i >= Env::IO_LOW; --i) {
+        auto& queue = queue_[i];
+        if (!queue.empty()) {
+          queue.front()->cv.Signal();
+          break;
         }
       }
     }