From 37ed64f647f981fe5ad12fd9ec35b23a8b01a7c8 Mon Sep 17 00:00:00 2001
From: jinhelin <linjinhe33@gmail.com>
Date: Tue, 11 Oct 2022 17:09:50 +0800
Subject: [PATCH] Fix I/O limiter auto-tuner. (#6098)

close pingcap/tiflash#6099
---
 dbms/src/Encryption/RateLimiter.cpp           | 31 +++++++++++++++++++
 dbms/src/Encryption/RateLimiter.h             |  5 +--
 .../Encryption/tests/gtest_rate_limiter.cpp   | 31 +++++++++++++++++++
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/dbms/src/Encryption/RateLimiter.cpp b/dbms/src/Encryption/RateLimiter.cpp
index 302d8cc4c22..e988d5d5a16 100644
--- a/dbms/src/Encryption/RateLimiter.cpp
+++ b/dbms/src/Encryption/RateLimiter.cpp
@@ -897,4 +897,35 @@ IOLimitTuner::Watermark IOLimitTuner::getWatermark(int pct) const
         return Watermark::Low;
     }
 }
+
+IOLimitTuner::Watermark IOLimitTuner::getWatermark(const LimiterStatUPtr & fg, const LimiterStatUPtr & bg, int pct) const
+{
+    // Take `bg_read` and `fg_read` for example:
+    // 1. Both `max_bg_read_bytes_per_sec` and `max_fg_read_bytes_per_sec` are less than `io_config.min_bytes_per_sec`.
+    // 2. `bg_read` runs out of the bandwidth quota, but `fg_read`'s bandwidth quota has not been used.
+    // 3. The usage rate of read is `(max_bg_read_bytes_per_sec + 0 ) / (max_bg_read_bytes_per_sec + max_fg_read_bytes_per_sec)`, about 50%.
+    // 4. 50% is less than `io_config.medium_pct`(60% by default), so watermark is `LOW`.
+    // 5. The `LOW` watermark means that bandwidth quota of read is sufficient since the usage rate is less than 60%, so it is unnessary to increase its bandwidth quota by decreasing the bandwidth quota of write.
+    // 6. `bg_read` will only try to increase its bandwidth quota by decreasing the bandwidth quota of `fg_read`.
+    // 7. However, `fg_read` is too small to decrease, so `bg_read` cannot be increased neither.
+    // 8. To avoid the bad case above, if the bandwidth quota we want to decrease is too small, returning the greater watermark and try to tune bandwidth between read and write.
+    if (fg != nullptr && bg != nullptr)
+    {
+        auto fg_wm = getWatermark(fg->pct());
+        auto bg_wm = getWatermark(bg->pct());
+        auto fg_mbps = fg->maxBytesPerSec();
+        auto bg_mbps = bg->maxBytesPerSec();
+        // `fg` needs more bandwidth, but `bg`'s bandwidth is small.
+        if (fg_wm > bg_wm && bg_mbps <= io_config.min_bytes_per_sec * 2)
+        {
+            return fg_wm;
+        }
+        // `bg_read` needs more bandwidth, but `fg_read`'s bandwidth is small.
+        else if (bg_wm > fg_wm && fg_mbps <= io_config.min_bytes_per_sec * 2)
+        {
+            return bg_wm;
+        }
+    }
+    return getWatermark(pct);
+}
 } // namespace DB
diff --git a/dbms/src/Encryption/RateLimiter.h b/dbms/src/Encryption/RateLimiter.h
index b0578433d58..25f82212602 100644
--- a/dbms/src/Encryption/RateLimiter.h
+++ b/dbms/src/Encryption/RateLimiter.h
@@ -412,13 +412,14 @@ class IOLimitTuner
     };
     Watermark writeWatermark() const
     {
-        return getWatermark(writePct());
+        return getWatermark(fg_write_stat, bg_write_stat, writePct());
     }
     Watermark readWatermark() const
     {
-        return getWatermark(readPct());
+        return getWatermark(fg_read_stat, bg_read_stat, readPct());
     }
     Watermark getWatermark(int pct) const;
+    Watermark getWatermark(const LimiterStatUPtr & fg, const LimiterStatUPtr & bg, int pct) const;
 
     // Returns <max_read_bytes_per_sec, max_write_bytes_per_sec, has_tuned>
     std::tuple<Int64, Int64, bool> tuneReadWrite() const;
diff --git a/dbms/src/Encryption/tests/gtest_rate_limiter.cpp b/dbms/src/Encryption/tests/gtest_rate_limiter.cpp
index 9913aab9dd7..613e552def5 100644
--- a/dbms/src/Encryption/tests/gtest_rate_limiter.cpp
+++ b/dbms/src/Encryption/tests/gtest_rate_limiter.cpp
@@ -731,5 +731,36 @@ TEST(IOLimitTunerTest, Tune)
     }
 }
 
+TEST(IOLimitTunerTest, Tune2)
+{
+    StorageIORateLimitConfig io_config;
+    io_config.max_bytes_per_sec = 2000;
+    io_config.min_bytes_per_sec = 10;
+
+    auto bg_write_stat = createLimiterStat(0, 1000, 1000, 990);
+    auto fg_write_stat = createLimiterStat(0, 1000, 1000, 990);
+    auto bg_read_stat = createLimiterStat(10, 1000, 1000, 10);
+    auto fg_read_stat = createLimiterStat(0, 1000, 1000, 10);
+
+    ASSERT_EQ(bg_write_stat->pct(), 0);
+    ASSERT_EQ(fg_write_stat->pct(), 0);
+    ASSERT_EQ(bg_read_stat->pct(), 100);
+    ASSERT_EQ(fg_read_stat->pct(), 0);
+    ASSERT_EQ(bg_read_stat->maxBytesPerSec(), 10);
+
+    IOLimitTuner tuner(
+        std::move(bg_write_stat),
+        std::move(fg_write_stat),
+        std::move(bg_read_stat),
+        std::move(fg_read_stat),
+        io_config);
+    ASSERT_EQ(tuner.readWatermark(), Emergency);
+
+    auto res = tuner.tune();
+    ASSERT_TRUE(res.write_tuned);
+    ASSERT_TRUE(res.read_tuned);
+    ASSERT_GT(res.max_bg_read_bytes_per_sec, 10);
+}
+
 } // namespace tests
 } // namespace DB