From: Andrew Kryczka <ajkr@users.noreply.github.com>
Date: Mon, 28 Aug 2023 20:36:25 +0000 (-0700)
Subject: Fix `GenericRateLimiter` hanging bug (#11763)
X-Git-Tag: v8.4.4~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c15ee5a758cd7c6168fae79360fcde65cb66251f;p=rocksdb.git

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
index 000000000..8f789e186
--- /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 be54138d9..ddb9bdbf0 100644
--- 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;
         }
       }
     }