Ignore canceled requests in the circuit breaker

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)) {

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;