From 7a85d1c633463e33d0481aeb035533b34528c606 Mon Sep 17 00:00:00 2001 From: wushinan Date: Tue, 8 Sep 2026 16:34:19 +0800 Subject: [PATCH] Ignore canceled requests in the circuit breaker --- 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 785ec77a5c..385c5a7bdc 100644 --- a/src/brpc/circuit_breaker.cpp +++ b/src/brpc/circuit_breaker.cpp @@ -18,6 +18,7 @@ #include "brpc/circuit_breaker.h" #include +#include #include #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 607fede9ea..975f3c654a 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 #include #include #include @@ -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 thread_list; std::vector> fc_list;