This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 2a75bbc3 Ignore canceled requests in the circuit breaker (#3532)
2a75bbc3 is described below
commit 2a75bbc3878c14505b92e580e6d0c7f9bcf3e686
Author: wushinanqiyi <[email protected]>
AuthorDate: Sun Sep 13 22:28:37 2026 +0800
Ignore canceled requests in the circuit breaker (#3532)
---
src/brpc/circuit_breaker.cpp | 5 +++-
test/brpc_circuit_breaker_unittest.cpp | 55 ++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/src/brpc/circuit_breaker.cpp b/src/brpc/circuit_breaker.cpp
index 785ec77a..385c5a7b 100644
--- a/src/brpc/circuit_breaker.cpp
+++ b/src/brpc/circuit_breaker.cpp
@@ -18,6 +18,7 @@
#include "brpc/circuit_breaker.h"
#include <cmath>
+#include <errno.h>
#include <gflags/gflags.h>
#include "brpc/errno.pb.h"
@@ -191,7 +192,9 @@ bool CircuitBreaker::OnCallEnd(int error_code, int64_t
latency) {
// since the latency corresponding to ELIMIT is usually very small, we
// cannot handle it as a successful request. Here we simply ignore the
requests
// that returned ELIMIT.
- if (error_code == ELIMIT) {
+ // Canceled requests do not indicate a server failure and should not
+ // contribute samples or count as successful half-open probes either.
+ if (error_code == ELIMIT || error_code == ECANCELED) {
return true;
}
if (_broken.load(butil::memory_order_relaxed)) {
diff --git a/test/brpc_circuit_breaker_unittest.cpp
b/test/brpc_circuit_breaker_unittest.cpp
index 607fede9..975f3c65 100644
--- a/test/brpc_circuit_breaker_unittest.cpp
+++ b/test/brpc_circuit_breaker_unittest.cpp
@@ -19,6 +19,7 @@
// Date: 2018/09/19 14:51:06
+#include <errno.h>
#include <pthread.h>
#include <gtest/gtest.h>
#include <gflags/gflags.h>
@@ -150,6 +151,60 @@ TEST_F(CircuitBreakerTest, should_not_isolate) {
}
}
+TEST_F(CircuitBreakerTest, canceled_requests_during_initialization) {
+ brpc::CircuitBreaker baseline;
+ for (int i = 0; i < 2 * kLongWindowSize; ++i) {
+ ASSERT_TRUE(_circuit_breaker.OnCallEnd(ECANCELED, kLatency));
+ }
+ EXPECT_EQ(0, _circuit_breaker.isolated_times());
+
+ // Cancellations must not advance initialization or consume its error
budget.
+ bool healthy = true;
+ for (int i = 0; i < kLongWindowSize && healthy; ++i) {
+ healthy = baseline.OnCallEnd(kErrorCodeForFailed, kErrorCost);
+ ASSERT_EQ(healthy,
+ _circuit_breaker.OnCallEnd(kErrorCodeForFailed, kErrorCost));
+ }
+ EXPECT_FALSE(healthy);
+ EXPECT_EQ(1, _circuit_breaker.isolated_times());
+}
+
+TEST_F(CircuitBreakerTest, canceled_requests_after_initialization) {
+ brpc::CircuitBreaker baseline;
+ for (int i = 0; i < 2 * kLongWindowSize; ++i) {
+ ASSERT_TRUE(baseline.OnCallEnd(0, kLatency));
+ ASSERT_TRUE(_circuit_breaker.OnCallEnd(0, kLatency));
+ }
+
+ // Interleaved cancellations must neither add error cost nor decay it
+ // like successful requests, regardless of their latency.
+ bool healthy = true;
+ for (int i = 0; i < 2 * kLongWindowSize && healthy; ++i) {
+ ASSERT_TRUE(_circuit_breaker.OnCallEnd(ECANCELED, 1));
+ ASSERT_TRUE(_circuit_breaker.OnCallEnd(ECANCELED, 100 * kLatency));
+ healthy = baseline.OnCallEnd(kErrorCodeForFailed, kErrorCost);
+ ASSERT_EQ(healthy,
+ _circuit_breaker.OnCallEnd(kErrorCodeForFailed, kErrorCost));
+ }
+ EXPECT_FALSE(healthy);
+ EXPECT_EQ(1, _circuit_breaker.isolated_times());
+}
+
+TEST_F(CircuitBreakerTest, canceled_requests_in_half_open) {
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FLAGS_circuit_breaker_half_open_window_size = 2;
+ _circuit_breaker.Reset();
+ ASSERT_TRUE(_circuit_breaker.OnCallEnd(0, kLatency));
+ for (int i = 0; i < 2 * kLongWindowSize; ++i) {
+ ASSERT_TRUE(_circuit_breaker.OnCallEnd(ECANCELED, kLatency));
+ }
+ EXPECT_EQ(0, _circuit_breaker.isolated_times());
+
+ // One successful probe is still missing: a real error must reopen it.
+ EXPECT_FALSE(_circuit_breaker.OnCallEnd(kErrorCodeForFailed, kErrorCost));
+ EXPECT_EQ(1, _circuit_breaker.isolated_times());
+}
+
TEST_F(CircuitBreakerTest, should_isolate) {
std::vector<pthread_t> thread_list;
std::vector<std::unique_ptr<FeedbackControl>> fc_list;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
