You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "feng-y (via GitHub)" <gi...@apache.org> on 2026/02/09 06:24:32 UTC

[PR] feat(auto_cl): add error rate threshold for punishment attenuation (brpc)

feng-y opened a new pull request, #3219:
URL: https://github.com/apache/brpc/pull/3219

   Add new GFlag `auto_cl_error_rate_punish_threshold` to enable error-rate-based punishment attenuation in AutoConcurrencyLimiter.
   
   Problem: Low error rates (e.g., 1.3% sporadic timeouts) cause disproportionate avg_latency inflation (+31%), leading the limiter to mistakenly shrink max_concurrency and trigger ELIMIT rejections.
   
   Solution: Inspired by Alibaba Sentinel's threshold-based approach:
   - threshold=0 (default): Original behavior preserved (backward compat)
   - threshold>0 (e.g., 0.1): Error rates below threshold produce zero punishment; above it, punishment scales linearly from 0 to full
   
   Example: With threshold=0.1, a 5% error rate produces no punishment, while a 50% error rate produces 44% of the original punishment.
   
   ### What problem does this PR solve?
   
   Issue Number: resolve 
   
   Problem Summary:
   
   ### What is changed and the side effects?
   
   Changed:
   
   Side effects:
   - Performance effects:
   
   - Breaking backward compatibility: 
   
   ---
   ### Check List:
   - Please make sure your changes are compilable.
   - When providing us with a new feature, it is best to add related tests.
   - Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


Re: [PR] feat(auto_cl): add error rate threshold for punishment attenuation (brpc)

Posted by "feng-y (via GitHub)" <gi...@apache.org>.
feng-y commented on PR #3219:
URL: https://github.com/apache/brpc/pull/3219#issuecomment-3872131518

   @wwbmmm fix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


Re: [PR] feat(auto_cl): add error rate threshold for punishment attenuation (brpc)

Posted by "TousakaRin (via GitHub)" <gi...@apache.org>.
TousakaRin merged PR #3219:
URL: https://github.com/apache/brpc/pull/3219


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


Re: [PR] feat(auto_cl): add error rate threshold for punishment attenuation (brpc)

Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #3219:
URL: https://github.com/apache/brpc/pull/3219#issuecomment-3888826126

   LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


Re: [PR] feat(auto_cl): add error rate threshold for punishment attenuation (brpc)

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3219:
URL: https://github.com/apache/brpc/pull/3219#discussion_r2782147137


##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,169 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"
+#include <gtest/gtest.h>
+

Review Comment:
   This new unit test file is picked up by CMake’s `file(GLOB BRPC_UNITTESTS "brpc_*_unittest.cpp")`, but it is not referenced by any Bazel `cc_test` target in `test/BUILD.bazel` (only `brpc_prometheus_*_unittest.cpp` are globbed). If Bazel is used in CI, this test won’t run; consider adding a Bazel target/glob so the coverage is consistent across build systems.



##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,169 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"
+#include <gtest/gtest.h>
+
+namespace brpc {
+namespace policy {
+
+DECLARE_int32(auto_cl_sample_window_size_ms);
+DECLARE_int32(auto_cl_min_sample_count);
+DECLARE_int32(auto_cl_max_sample_count);
+DECLARE_bool(auto_cl_enable_error_punish);
+DECLARE_double(auto_cl_fail_punish_ratio);
+DECLARE_double(auto_cl_error_rate_punish_threshold);
+
+}  // namespace policy
+}  // namespace brpc
+
+class AutoConcurrencyLimiterTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        // Save original values
+        orig_sample_window_size_ms_ = brpc::policy::FLAGS_auto_cl_sample_window_size_ms;
+        orig_min_sample_count_ = brpc::policy::FLAGS_auto_cl_min_sample_count;
+        orig_max_sample_count_ = brpc::policy::FLAGS_auto_cl_max_sample_count;
+        orig_enable_error_punish_ = brpc::policy::FLAGS_auto_cl_enable_error_punish;
+        orig_fail_punish_ratio_ = brpc::policy::FLAGS_auto_cl_fail_punish_ratio;
+        orig_error_rate_threshold_ = brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold;
+
+        // Set test-friendly values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 1000;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = 5;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = 200;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = true;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = 1.0;
+    }
+
+    void TearDown() override {
+        // Restore original values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = orig_sample_window_size_ms_;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = orig_min_sample_count_;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = orig_max_sample_count_;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = orig_enable_error_punish_;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = orig_fail_punish_ratio_;
+        brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = orig_error_rate_threshold_;
+    }
+
+private:
+    int32_t orig_sample_window_size_ms_;
+    int32_t orig_min_sample_count_;
+    int32_t orig_max_sample_count_;
+    bool orig_enable_error_punish_;
+    double orig_fail_punish_ratio_;
+    double orig_error_rate_threshold_;
+};
+
+// Helper function to add samples and trigger window completion
+// Uses synthetic timestamps instead of sleeping for faster, deterministic tests.
+// The final successful sample is used as the trigger, so actual counts match
+// succ_count/fail_count exactly (preserving intended error rates).
+void AddSamplesAndTriggerWindow(brpc::policy::AutoConcurrencyLimiter& limiter,
+                                 int succ_count, int64_t succ_latency,
+                                 int fail_count, int64_t fail_latency) {
+    ASSERT_GT(succ_count, 0) << "Need at least 1 success to trigger window";
+    int64_t now = butil::gettimeofday_us();
+
+    // Add successful samples (reserve one for the trigger)
+    for (int i = 0; i < succ_count - 1; ++i) {
+        limiter.AddSample(0, succ_latency, now);
+    }
+    // Add failed samples
+    for (int i = 0; i < fail_count; ++i) {
+        limiter.AddSample(1, fail_latency, now);
+    }
+
+    // Advance timestamp past window expiry instead of sleeping
+    int64_t after_window = now + brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000;
+
+    // Use the final success sample to trigger window submission
+    limiter.AddSample(0, succ_latency, after_window);
+}
+
+// Test 1: Backward compatibility - threshold=0 preserves original punishment behavior
+TEST_F(AutoConcurrencyLimiterTest, ThresholdZeroPreservesOriginalBehavior) {
+    brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = 0;
+    brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 10;
+
+    brpc::policy::AutoConcurrencyLimiter limiter;
+    AddSamplesAndTriggerWindow(limiter, 90, 100, 10, 1000);
+
+    // 10% error rate, threshold=0 means full punishment applied
+    // avg_latency = (10*1000 + 90*100) / 90 = 211us

Review Comment:
   The expected-value comment here ignores the `std::ceil(...)` used in `AutoConcurrencyLimiter::UpdateMaxConcurrency`, so the documented average is off by 1us (19000/90 → 212, not 211). Consider updating the comment to match the actual rounding behavior to reduce confusion when maintaining the test.
   ```suggestion
       // avg_latency = (10*1000 + 90*100) / 90 ≈ 212us (after std::ceil)
   ```



##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,169 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"
+#include <gtest/gtest.h>
+
+namespace brpc {
+namespace policy {
+
+DECLARE_int32(auto_cl_sample_window_size_ms);
+DECLARE_int32(auto_cl_min_sample_count);
+DECLARE_int32(auto_cl_max_sample_count);
+DECLARE_bool(auto_cl_enable_error_punish);
+DECLARE_double(auto_cl_fail_punish_ratio);
+DECLARE_double(auto_cl_error_rate_punish_threshold);
+
+}  // namespace policy
+}  // namespace brpc
+
+class AutoConcurrencyLimiterTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        // Save original values
+        orig_sample_window_size_ms_ = brpc::policy::FLAGS_auto_cl_sample_window_size_ms;
+        orig_min_sample_count_ = brpc::policy::FLAGS_auto_cl_min_sample_count;
+        orig_max_sample_count_ = brpc::policy::FLAGS_auto_cl_max_sample_count;
+        orig_enable_error_punish_ = brpc::policy::FLAGS_auto_cl_enable_error_punish;
+        orig_fail_punish_ratio_ = brpc::policy::FLAGS_auto_cl_fail_punish_ratio;
+        orig_error_rate_threshold_ = brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold;
+
+        // Set test-friendly values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 1000;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = 5;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = 200;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = true;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = 1.0;
+    }
+
+    void TearDown() override {
+        // Restore original values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = orig_sample_window_size_ms_;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = orig_min_sample_count_;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = orig_max_sample_count_;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = orig_enable_error_punish_;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = orig_fail_punish_ratio_;
+        brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = orig_error_rate_threshold_;
+    }
+
+private:
+    int32_t orig_sample_window_size_ms_;
+    int32_t orig_min_sample_count_;
+    int32_t orig_max_sample_count_;
+    bool orig_enable_error_punish_;
+    double orig_fail_punish_ratio_;
+    double orig_error_rate_threshold_;
+};
+
+// Helper function to add samples and trigger window completion
+// Uses synthetic timestamps instead of sleeping for faster, deterministic tests.
+// The final successful sample is used as the trigger, so actual counts match
+// succ_count/fail_count exactly (preserving intended error rates).
+void AddSamplesAndTriggerWindow(brpc::policy::AutoConcurrencyLimiter& limiter,
+                                 int succ_count, int64_t succ_latency,
+                                 int fail_count, int64_t fail_latency) {
+    ASSERT_GT(succ_count, 0) << "Need at least 1 success to trigger window";
+    int64_t now = butil::gettimeofday_us();
+
+    // Add successful samples (reserve one for the trigger)
+    for (int i = 0; i < succ_count - 1; ++i) {
+        limiter.AddSample(0, succ_latency, now);
+    }
+    // Add failed samples
+    for (int i = 0; i < fail_count; ++i) {
+        limiter.AddSample(1, fail_latency, now);
+    }
+
+    // Advance timestamp past window expiry instead of sleeping
+    int64_t after_window = now + brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000;
+
+    // Use the final success sample to trigger window submission
+    limiter.AddSample(0, succ_latency, after_window);
+}
+
+// Test 1: Backward compatibility - threshold=0 preserves original punishment behavior
+TEST_F(AutoConcurrencyLimiterTest, ThresholdZeroPreservesOriginalBehavior) {
+    brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = 0;
+    brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 10;
+
+    brpc::policy::AutoConcurrencyLimiter limiter;
+    AddSamplesAndTriggerWindow(limiter, 90, 100, 10, 1000);
+
+    // 10% error rate, threshold=0 means full punishment applied
+    // avg_latency = (10*1000 + 90*100) / 90 = 211us
+    ASSERT_GT(limiter._min_latency_us, 180);
+    ASSERT_LT(limiter._min_latency_us, 250);
+}
+
+// Test 2: Dead zone - error rate below threshold produces zero punishment
+TEST_F(AutoConcurrencyLimiterTest, BelowThresholdZeroPunishment) {
+    brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = 0.2;  // 20% threshold
+    brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 10;
+
+    brpc::policy::AutoConcurrencyLimiter limiter;
+    AddSamplesAndTriggerWindow(limiter, 90, 100, 10, 1000);
+
+    // 10% error rate < 20% threshold, punishment should be zero
+    // avg_latency = 90*100 / 90 = 100us (no inflation)
+    ASSERT_GT(limiter._min_latency_us, 80);
+    ASSERT_LT(limiter._min_latency_us, 130);
+}
+
+// Test 3: Boundary - error rate exactly at threshold produces zero punishment
+TEST_F(AutoConcurrencyLimiterTest, ExactlyAtThresholdZeroPunishment) {
+    brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = 0.1;  // 10% threshold
+    brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 10;
+
+    brpc::policy::AutoConcurrencyLimiter limiter;
+    AddSamplesAndTriggerWindow(limiter, 90, 100, 10, 1000);
+
+    // 10% error rate == 10% threshold, punishment should be zero
+    // avg_latency = 90*100 / 90 = 100us
+    ASSERT_GT(limiter._min_latency_us, 80);
+    ASSERT_LT(limiter._min_latency_us, 130);
+}
+
+// Test 4: Linear scaling - above threshold, punishment scales proportionally
+TEST_F(AutoConcurrencyLimiterTest, AboveThresholdLinearScaling) {
+    brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = 0.1;  // 10% threshold
+    brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 10;
+
+    // Case A: 50% error rate
+    // punish_factor = (0.5 - 0.1) / (1.0 - 0.1) = 0.444
+    // failed_punish = 50 * 1000 * 0.444 = 22222us
+    // avg_latency = (22222 + 50*100) / 50 = 544us

Review Comment:
   The expected-value comment here ignores the `std::ceil(...)` used in `AutoConcurrencyLimiter::UpdateMaxConcurrency`, so the documented average is off by 1us ((22222.2+5000)/50 → 545, not 544). Consider updating the comment to match the actual rounding behavior.
   ```suggestion
       // avg_latency = (22222 + 50*100) / 50 = 545us
   ```



##########
src/brpc/policy/auto_concurrency_limiter.cpp:
##########
@@ -236,7 +244,27 @@ void AutoConcurrencyLimiter::AdjustMaxConcurrency(int next_max_concurrency) {
 void AutoConcurrencyLimiter::UpdateMaxConcurrency(int64_t sampling_time_us) {
     int32_t total_succ_req = _total_succ_req.load(butil::memory_order_relaxed);
     double failed_punish = _sw.total_failed_us * FLAGS_auto_cl_fail_punish_ratio;
-    int64_t avg_latency = 
+
+    // Threshold-based attenuation: when auto_cl_error_rate_punish_threshold > 0,
+    // attenuate punishment based on error rate. Inspired by Sentinel's threshold-
+    // based circuit breaker: low error rates should not inflate avg_latency.
+    // Above threshold, punishment scales linearly from 0 to full strength.
+    // When threshold is 0 (default), this block is skipped entirely.
+    if (FLAGS_auto_cl_error_rate_punish_threshold > 0 && _sw.failed_count > 0) {
+        double threshold = FLAGS_auto_cl_error_rate_punish_threshold;
+        double error_rate = static_cast<double>(_sw.failed_count) /
+            (_sw.succ_count + _sw.failed_count);
+        if (error_rate <= threshold) {
+            // Error rate within dead zone, cancel punishment.
+            failed_punish = 0;
+        } else {
+            // Linear ramp: 0 at threshold, 1.0 at 100% error rate.
+            double punish_factor = (error_rate - threshold) / (1.0 - threshold);
+            failed_punish *= punish_factor;
+        }

Review Comment:
   `auto_cl_error_rate_punish_threshold` is used as a probability/ratio but there’s no validation of its range. Values <0 or >1 can lead to surprising behavior (e.g. thresholds >1 disable punishment entirely; negative thresholds change the ramp), and the formula relies on `1.0 - threshold` being meaningful. Consider enforcing `0 <= threshold && threshold < 1` via a gflags validator (e.g. `BUTIL_VALIDATE_GFLAG`) or clamping/early-returning when out of range, and update the flag help string to state the valid range.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


Re: [PR] feat(auto_cl): add error rate threshold for punishment attenuation (brpc)

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3219:
URL: https://github.com/apache/brpc/pull/3219#discussion_r2782261739


##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,169 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"
+#include <gtest/gtest.h>
+
+namespace brpc {
+namespace policy {
+
+DECLARE_int32(auto_cl_sample_window_size_ms);
+DECLARE_int32(auto_cl_min_sample_count);
+DECLARE_int32(auto_cl_max_sample_count);
+DECLARE_bool(auto_cl_enable_error_punish);
+DECLARE_double(auto_cl_fail_punish_ratio);
+DECLARE_double(auto_cl_error_rate_punish_threshold);
+
+}  // namespace policy
+}  // namespace brpc
+
+class AutoConcurrencyLimiterTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        // Save original values
+        orig_sample_window_size_ms_ = brpc::policy::FLAGS_auto_cl_sample_window_size_ms;
+        orig_min_sample_count_ = brpc::policy::FLAGS_auto_cl_min_sample_count;
+        orig_max_sample_count_ = brpc::policy::FLAGS_auto_cl_max_sample_count;
+        orig_enable_error_punish_ = brpc::policy::FLAGS_auto_cl_enable_error_punish;
+        orig_fail_punish_ratio_ = brpc::policy::FLAGS_auto_cl_fail_punish_ratio;
+        orig_error_rate_threshold_ = brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold;
+
+        // Set test-friendly values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 1000;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = 5;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = 200;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = true;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = 1.0;
+    }
+
+    void TearDown() override {
+        // Restore original values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = orig_sample_window_size_ms_;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = orig_min_sample_count_;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = orig_max_sample_count_;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = orig_enable_error_punish_;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = orig_fail_punish_ratio_;
+        brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = orig_error_rate_threshold_;
+    }
+
+private:
+    int32_t orig_sample_window_size_ms_;
+    int32_t orig_min_sample_count_;
+    int32_t orig_max_sample_count_;
+    bool orig_enable_error_punish_;
+    double orig_fail_punish_ratio_;
+    double orig_error_rate_threshold_;
+};
+
+// Helper function to add samples and trigger window completion
+// Uses synthetic timestamps instead of sleeping for faster, deterministic tests.
+// The final successful sample is used as the trigger, so actual counts match
+// succ_count/fail_count exactly (preserving intended error rates).
+void AddSamplesAndTriggerWindow(brpc::policy::AutoConcurrencyLimiter& limiter,
+                                 int succ_count, int64_t succ_latency,
+                                 int fail_count, int64_t fail_latency) {
+    ASSERT_GT(succ_count, 0) << "Need at least 1 success to trigger window";
+    int64_t now = butil::gettimeofday_us();
+
+    // Add successful samples (reserve one for the trigger)
+    for (int i = 0; i < succ_count - 1; ++i) {
+        limiter.AddSample(0, succ_latency, now);
+    }
+    // Add failed samples
+    for (int i = 0; i < fail_count; ++i) {
+        limiter.AddSample(1, fail_latency, now);
+    }
+
+    // Advance timestamp past window expiry instead of sleeping
+    int64_t after_window = now + brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000;
+
+    // Use the final success sample to trigger window submission
+    limiter.AddSample(0, succ_latency, after_window);
+}
+
+// Test 1: Backward compatibility - threshold=0 preserves original punishment behavior
+TEST_F(AutoConcurrencyLimiterTest, ThresholdZeroPreservesOriginalBehavior) {
+    brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = 0;
+    brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 10;
+
+    brpc::policy::AutoConcurrencyLimiter limiter;
+    AddSamplesAndTriggerWindow(limiter, 90, 100, 10, 1000);
+
+    // 10% error rate, threshold=0 means full punishment applied
+    // avg_latency = ceil((10*1000 + 90*100) / 90) = ceil(211.1) = 212us
+    ASSERT_GT(limiter._min_latency_us, 180);
+    ASSERT_LT(limiter._min_latency_us, 250);

Review Comment:
   This test directly reads `limiter._min_latency_us`, but `_min_latency_us` is a private member of `AutoConcurrencyLimiter`, so the test will not compile. Please access this via a public/testing accessor (e.g., a `min_latency_us_for_test()` under `#ifdef UNIT_TEST` or `FRIEND_TEST`), or assert behavior through a public API instead of private state.



##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,169 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"

Review Comment:
   `#include "bthread/bthread.h"` appears unused in this test file. Removing unused includes helps keep build time and dependencies minimal.
   ```suggestion
   
   ```



##########
src/brpc/policy/auto_concurrency_limiter.cpp:
##########
@@ -77,6 +77,14 @@ DEFINE_int32(auto_cl_latency_fluctuation_correction_factor, 1,
              "the value, the higher the tolerance for the fluctuation of the "
              "latency. If the value is too large, the latency will be higher "
              "when the server is overloaded.");
+DEFINE_double(auto_cl_error_rate_punish_threshold, 0,
+              "Threshold for error-rate-based punishment attenuation. "
+              "Valid range: (0, 1). Values outside this range are ignored "
+              "and original punishment logic is used. "
+              "0 (default): no effect, original punishment logic is used. "
+              "e.g. 0.1: error rates below 10%% produce zero punishment; "
+              "above it the punishment scales linearly from 0 to full strength. "

Review Comment:
   The flag help text says "Valid range: (0, 1)", but `0` is also an explicitly supported value (disable / preserve original behavior). To avoid confusion for operators, consider updating the wording to something like "Valid range: [0, 1) (0 disables)".



##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,169 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"
+#include <gtest/gtest.h>
+
+namespace brpc {
+namespace policy {
+
+DECLARE_int32(auto_cl_sample_window_size_ms);
+DECLARE_int32(auto_cl_min_sample_count);
+DECLARE_int32(auto_cl_max_sample_count);
+DECLARE_bool(auto_cl_enable_error_punish);
+DECLARE_double(auto_cl_fail_punish_ratio);
+DECLARE_double(auto_cl_error_rate_punish_threshold);
+
+}  // namespace policy
+}  // namespace brpc
+
+class AutoConcurrencyLimiterTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        // Save original values
+        orig_sample_window_size_ms_ = brpc::policy::FLAGS_auto_cl_sample_window_size_ms;
+        orig_min_sample_count_ = brpc::policy::FLAGS_auto_cl_min_sample_count;
+        orig_max_sample_count_ = brpc::policy::FLAGS_auto_cl_max_sample_count;
+        orig_enable_error_punish_ = brpc::policy::FLAGS_auto_cl_enable_error_punish;
+        orig_fail_punish_ratio_ = brpc::policy::FLAGS_auto_cl_fail_punish_ratio;
+        orig_error_rate_threshold_ = brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold;
+
+        // Set test-friendly values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 1000;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = 5;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = 200;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = true;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = 1.0;
+    }
+
+    void TearDown() override {
+        // Restore original values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = orig_sample_window_size_ms_;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = orig_min_sample_count_;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = orig_max_sample_count_;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = orig_enable_error_punish_;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = orig_fail_punish_ratio_;
+        brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = orig_error_rate_threshold_;
+    }
+
+private:
+    int32_t orig_sample_window_size_ms_;
+    int32_t orig_min_sample_count_;
+    int32_t orig_max_sample_count_;
+    bool orig_enable_error_punish_;
+    double orig_fail_punish_ratio_;
+    double orig_error_rate_threshold_;
+};
+
+// Helper function to add samples and trigger window completion
+// Uses synthetic timestamps instead of sleeping for faster, deterministic tests.
+// The final successful sample is used as the trigger, so actual counts match
+// succ_count/fail_count exactly (preserving intended error rates).
+void AddSamplesAndTriggerWindow(brpc::policy::AutoConcurrencyLimiter& limiter,
+                                 int succ_count, int64_t succ_latency,
+                                 int fail_count, int64_t fail_latency) {
+    ASSERT_GT(succ_count, 0) << "Need at least 1 success to trigger window";
+    int64_t now = butil::gettimeofday_us();
+
+    // Add successful samples (reserve one for the trigger)
+    for (int i = 0; i < succ_count - 1; ++i) {
+        limiter.AddSample(0, succ_latency, now);
+    }
+    // Add failed samples
+    for (int i = 0; i < fail_count; ++i) {
+        limiter.AddSample(1, fail_latency, now);
+    }
+
+    // Advance timestamp past window expiry instead of sleeping
+    int64_t after_window = now + brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000;
+
+    // Use the final success sample to trigger window submission
+    limiter.AddSample(0, succ_latency, after_window);

Review Comment:
   `AutoConcurrencyLimiter::AddSample(...)` is a private method (declared in auto_concurrency_limiter.h), so this helper will not compile as written. Consider driving the limiter via the public `OnResponded(...)` API, or (preferably for deterministic testing) exposing a test-only hook (e.g., `#ifdef UNIT_TEST` method) or adding `FRIEND_TEST` declarations in `AutoConcurrencyLimiter` to allow tests to call `AddSample` with synthetic timestamps.
   ```suggestion
   // Drives the limiter via its public OnResponded API instead of the private
   // AddSample method, to respect encapsulation and keep tests compiling.
   // The final successful sample is used as the trigger, so actual counts match
   // succ_count/fail_count exactly (preserving intended error rates).
   void AddSamplesAndTriggerWindow(brpc::policy::AutoConcurrencyLimiter& limiter,
                                    int succ_count, int64_t succ_latency,
                                    int fail_count, int64_t fail_latency) {
       ASSERT_GT(succ_count, 0) << "Need at least 1 success to trigger window";
   
       // Add successful samples (reserve one for the trigger)
       for (int i = 0; i < succ_count - 1; ++i) {
           limiter.OnResponded(0, succ_latency);
       }
       // Add failed samples
       for (int i = 0; i < fail_count; ++i) {
           limiter.OnResponded(1, fail_latency);
       }
   
       // Wait for longer than the sample window so that the limiter will
       // treat the next sample as belonging to a new window.
       bthread_usleep(brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000);
   
       // Use the final success sample to trigger window submission
       limiter.OnResponded(0, succ_latency);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org


Re: [PR] feat(auto_cl): add error rate threshold for punishment attenuation (brpc)

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3219:
URL: https://github.com/apache/brpc/pull/3219#discussion_r2781390260


##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,208 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"
+#include <gtest/gtest.h>
+
+namespace brpc {
+namespace policy {
+
+DECLARE_int32(auto_cl_sample_window_size_ms);
+DECLARE_int32(auto_cl_min_sample_count);
+DECLARE_int32(auto_cl_max_sample_count);
+DECLARE_bool(auto_cl_enable_error_punish);
+DECLARE_double(auto_cl_fail_punish_ratio);
+DECLARE_double(auto_cl_error_rate_punish_threshold);
+
+}  // namespace policy
+}  // namespace brpc
+
+class AutoConcurrencyLimiterTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        // Save original values
+        orig_sample_window_size_ms_ = brpc::policy::FLAGS_auto_cl_sample_window_size_ms;
+        orig_min_sample_count_ = brpc::policy::FLAGS_auto_cl_min_sample_count;
+        orig_max_sample_count_ = brpc::policy::FLAGS_auto_cl_max_sample_count;
+        orig_enable_error_punish_ = brpc::policy::FLAGS_auto_cl_enable_error_punish;
+        orig_fail_punish_ratio_ = brpc::policy::FLAGS_auto_cl_fail_punish_ratio;
+        orig_error_rate_threshold_ = brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold;
+
+        // Set test-friendly values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 1000;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = 5;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = 200;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = true;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = 1.0;
+    }
+
+    void TearDown() override {
+        // Restore original values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = orig_sample_window_size_ms_;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = orig_min_sample_count_;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = orig_max_sample_count_;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = orig_enable_error_punish_;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = orig_fail_punish_ratio_;
+        brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = orig_error_rate_threshold_;
+    }
+
+private:
+    int32_t orig_sample_window_size_ms_;
+    int32_t orig_min_sample_count_;
+    int32_t orig_max_sample_count_;
+    bool orig_enable_error_punish_;
+    double orig_fail_punish_ratio_;
+    double orig_error_rate_threshold_;
+};
+
+// Helper function to add samples and trigger window completion
+void AddSamplesAndTriggerWindow(brpc::policy::AutoConcurrencyLimiter& limiter,
+                                 int succ_count, int64_t succ_latency,
+                                 int fail_count, int64_t fail_latency) {
+    int64_t now = butil::gettimeofday_us();
+
+    // Add successful samples
+    for (int i = 0; i < succ_count; ++i) {
+        limiter.AddSample(0, succ_latency, now);
+    }
+    // Add failed samples
+    for (int i = 0; i < fail_count; ++i) {
+        limiter.AddSample(1, fail_latency, now);
+    }
+
+    // Wait for window to expire and trigger update
+    bthread_usleep(brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000);
+
+    // Add one more sample to trigger window submission
+    limiter.AddSample(0, succ_latency, butil::gettimeofday_us());

Review Comment:
   `AddSamplesAndTriggerWindow` sleeps to advance the sampling window. Since the test is already calling `AddSample(error_code, latency_us, sampling_time_us)` directly (with an explicit timestamp parameter), consider advancing `sampling_time_us` instead of sleeping. This will make the unit tests faster and less timing-dependent/flaky under loaded CI machines.
   ```suggestion
       // Advance sampling time beyond the window size to trigger update,
       // without actually sleeping.
       int64_t trigger_time =
               now + static_cast<int64_t>(brpc::policy::FLAGS_auto_cl_sample_window_size_ms) * 1000
               + 1000;
   
       // Add one more sample to trigger window submission
       limiter.AddSample(0, succ_latency, trigger_time);
   ```



##########
test/brpc_auto_concurrency_limiter_unittest.cpp:
##########
@@ -0,0 +1,208 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/auto_concurrency_limiter.h"
+#include "butil/time.h"
+#include "bthread/bthread.h"
+#include <gtest/gtest.h>
+
+namespace brpc {
+namespace policy {
+
+DECLARE_int32(auto_cl_sample_window_size_ms);
+DECLARE_int32(auto_cl_min_sample_count);
+DECLARE_int32(auto_cl_max_sample_count);
+DECLARE_bool(auto_cl_enable_error_punish);
+DECLARE_double(auto_cl_fail_punish_ratio);
+DECLARE_double(auto_cl_error_rate_punish_threshold);
+
+}  // namespace policy
+}  // namespace brpc
+
+class AutoConcurrencyLimiterTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        // Save original values
+        orig_sample_window_size_ms_ = brpc::policy::FLAGS_auto_cl_sample_window_size_ms;
+        orig_min_sample_count_ = brpc::policy::FLAGS_auto_cl_min_sample_count;
+        orig_max_sample_count_ = brpc::policy::FLAGS_auto_cl_max_sample_count;
+        orig_enable_error_punish_ = brpc::policy::FLAGS_auto_cl_enable_error_punish;
+        orig_fail_punish_ratio_ = brpc::policy::FLAGS_auto_cl_fail_punish_ratio;
+        orig_error_rate_threshold_ = brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold;
+
+        // Set test-friendly values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = 1000;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = 5;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = 200;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = true;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = 1.0;
+    }
+
+    void TearDown() override {
+        // Restore original values
+        brpc::policy::FLAGS_auto_cl_sample_window_size_ms = orig_sample_window_size_ms_;
+        brpc::policy::FLAGS_auto_cl_min_sample_count = orig_min_sample_count_;
+        brpc::policy::FLAGS_auto_cl_max_sample_count = orig_max_sample_count_;
+        brpc::policy::FLAGS_auto_cl_enable_error_punish = orig_enable_error_punish_;
+        brpc::policy::FLAGS_auto_cl_fail_punish_ratio = orig_fail_punish_ratio_;
+        brpc::policy::FLAGS_auto_cl_error_rate_punish_threshold = orig_error_rate_threshold_;
+    }
+
+private:
+    int32_t orig_sample_window_size_ms_;
+    int32_t orig_min_sample_count_;
+    int32_t orig_max_sample_count_;
+    bool orig_enable_error_punish_;
+    double orig_fail_punish_ratio_;
+    double orig_error_rate_threshold_;
+};
+
+// Helper function to add samples and trigger window completion
+void AddSamplesAndTriggerWindow(brpc::policy::AutoConcurrencyLimiter& limiter,
+                                 int succ_count, int64_t succ_latency,
+                                 int fail_count, int64_t fail_latency) {
+    int64_t now = butil::gettimeofday_us();
+
+    // Add successful samples
+    for (int i = 0; i < succ_count; ++i) {
+        limiter.AddSample(0, succ_latency, now);
+    }
+    // Add failed samples
+    for (int i = 0; i < fail_count; ++i) {
+        limiter.AddSample(1, fail_latency, now);
+    }
+
+    // Wait for window to expire and trigger update
+    bthread_usleep(brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000);
+
+    // Add one more sample to trigger window submission
+    limiter.AddSample(0, succ_latency, butil::gettimeofday_us());

Review Comment:
   `AddSamplesAndTriggerWindow` always adds an extra successful sample (the final `AddSample(0, ...)` used to trigger submission). This changes the requested `succ_count`/`fail_count` totals and skews the effective error rate used by the assertions and the formulas in comments (e.g., the “90% error rate” case becomes 90/(90+10+1)). Consider restructuring the helper so the trigger call is included in `succ_count`/`fail_count` (e.g., add `succ_count - 1` successes before advancing the window, then use the final success as the trigger) so the test inputs match the intended rates.
   ```suggestion
       // We want the total number of successes and failures, including the
       // final trigger sample, to match succ_count and fail_count.
       // Reserve one sample (success or failure) as the trigger if possible.
       int pre_succ_count = succ_count;
       int pre_fail_count = fail_count;
       bool trigger_is_success = true;
   
       if (succ_count > 0) {
           // Use a success as the trigger: send one fewer success before the window.
           pre_succ_count = succ_count - 1;
           trigger_is_success = true;
       } else if (fail_count > 0) {
           // No successes requested; use a failure as the trigger instead.
           pre_fail_count = fail_count - 1;
           trigger_is_success = false;
       } else {
           // Edge case: no samples requested; fall back to previous behavior
           // of sending a single success solely to trigger the window.
           pre_succ_count = 0;
           pre_fail_count = 0;
           trigger_is_success = true;
       }
   
       // Add successful samples (excluding any reserved trigger sample)
       for (int i = 0; i < pre_succ_count; ++i) {
           limiter.AddSample(0, succ_latency, now);
       }
       // Add failed samples (excluding any reserved trigger sample)
       for (int i = 0; i < pre_fail_count; ++i) {
           limiter.AddSample(1, fail_latency, now);
       }
   
       // Wait for window to expire and trigger update
       bthread_usleep(brpc::policy::FLAGS_auto_cl_sample_window_size_ms * 1000 + 1000);
   
       // Add the final sample to trigger window submission.
       // If any samples were requested, this trigger is counted in
       // succ_count/fail_count; otherwise, we add a single success as before.
       if (succ_count + fail_count > 0) {
           if (trigger_is_success) {
               limiter.AddSample(0, succ_latency, butil::gettimeofday_us());
           } else {
               limiter.AddSample(1, fail_latency, butil::gettimeofday_us());
           }
       } else {
           limiter.AddSample(0, succ_latency, butil::gettimeofday_us());
       }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org