revert last_revived_time of circuit_breaker

src/brpc/circuit_breaker.cpp

@@ -164,10 +164,9 @@ CircuitBreaker::CircuitBreaker()
                    FLAGS_circuit_breaker_long_window_error_percent)
     , _short_window(FLAGS_circuit_breaker_short_window_size,
                     FLAGS_circuit_breaker_short_window_error_percent)
-    , _last_revived_time_ms(butil::cpuwide_time_ms())
+    , _last_reset_time_ms(0)
     , _isolation_duration_ms(FLAGS_circuit_breaker_min_isolation_duration_ms)
     , _isolated_times(0)
-    , _is_first_call_after_revived(true)
     , _broken(false) {
 }
 
@@ -175,10 +174,6 @@ bool CircuitBreaker::OnCallEnd(int error_code, int64_t latency) {
     if (_broken.load(butil::memory_order_relaxed)) {
         return false;
     }
-    if (_is_first_call_after_revived.load(butil::memory_order_relaxed) &&
-        _is_first_call_after_revived.exchange(false, butil::memory_order_relaxed)) {
-      _last_revived_time_ms.store(butil::cpuwide_time_ms(), butil::memory_order_relaxed);
-    }
     if (_long_window.OnCallEnd(error_code, latency) &&
         _short_window.OnCallEnd(error_code, latency)) {
         return true;
@@ -190,8 +185,7 @@ bool CircuitBreaker::OnCallEnd(int error_code, int64_t latency) {
 void CircuitBreaker::Reset() {
     _long_window.Reset();
     _short_window.Reset();
-    _last_revived_time_ms.store(butil::cpuwide_time_ms(), butil::memory_order_relaxed);
-    _is_first_call_after_revived.store(true, butil::memory_order_relaxed);
+    _last_reset_time_ms = butil::cpuwide_time_ms();
     _broken.store(false, butil::memory_order_release);
 }
 
@@ -209,7 +203,7 @@ void CircuitBreaker::UpdateIsolationDuration() {
         FLAGS_circuit_breaker_max_isolation_duration_ms;
     const int min_isolation_duration_ms =
         FLAGS_circuit_breaker_min_isolation_duration_ms;
-    if (now_time_ms - _last_revived_time_ms < max_isolation_duration_ms) {
+    if (now_time_ms - _last_reset_time_ms < max_isolation_duration_ms) {
         isolation_duration_ms =
             std::min(isolation_duration_ms * 2, max_isolation_duration_ms);
     } else {

src/brpc/circuit_breaker.h

@@ -82,7 +82,7 @@ private:
 
     EmaErrorRecorder _long_window;
     EmaErrorRecorder _short_window;
-    butil::atomic<int64_t> _last_revived_time_ms;
+    int64_t _last_reset_time_ms;
     butil::atomic<int> _isolation_duration_ms;
     butil::atomic<int> _isolated_times;
     butil::atomic<bool> _is_first_call_after_revived;

test/brpc_circuit_breaker_unittest.cpp

@@ -124,7 +124,7 @@ TEST_F(CircuitBreakerTest, should_not_isolate) {
     StartFeedbackThread(&thread_list, &fc_list, 3);
     for (int  i = 0; i < kThreadNum; ++i) {
         void* ret_data = nullptr;
-        EXPECT_EQ(pthread_join(thread_list[i], &ret_data), 0);
+        ASSERT_EQ(pthread_join(thread_list[i], &ret_data), 0);
         FeedbackControl* fc = static_cast<FeedbackControl*>(ret_data);
         EXPECT_EQ(fc->_unhealthy_cnt, 0);
         EXPECT_TRUE(fc->_healthy);
@@ -137,84 +137,84 @@ TEST_F(CircuitBreakerTest, should_isolate) {
     StartFeedbackThread(&thread_list, &fc_list, 50);
     for (int  i = 0; i < kThreadNum; ++i) {
         void* ret_data = nullptr;
-        EXPECT_EQ(pthread_join(thread_list[i], &ret_data), 0);
+        ASSERT_EQ(pthread_join(thread_list[i], &ret_data), 0);
         FeedbackControl* fc = static_cast<FeedbackControl*>(ret_data);
         EXPECT_GT(fc->_unhealthy_cnt, 0);
         EXPECT_FALSE(fc->_healthy);
     }
 }
 
-TEST_F(CircuitBreakerTest, isolation_duration_grow) {
-    _circuit_breaker.Reset();
+TEST_F(CircuitBreakerTest, isolation_duration_grow_and_reset) {
     std::vector<pthread_t> thread_list;
     std::vector<std::unique_ptr<FeedbackControl>> fc_list;
     StartFeedbackThread(&thread_list, &fc_list, 100);
     for (int  i = 0; i < kThreadNum; ++i) {
         void* ret_data = nullptr;
-        EXPECT_EQ(pthread_join(thread_list[i], &ret_data), 0);
+        ASSERT_EQ(pthread_join(thread_list[i], &ret_data), 0);
         FeedbackControl* fc = static_cast<FeedbackControl*>(ret_data);
         EXPECT_FALSE(fc->_healthy);
         EXPECT_LE(fc->_healthy_cnt, kShortWindowSize);
         EXPECT_GT(fc->_unhealthy_cnt, 0);
     }
-    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), kMinIsolationDurationMs * 2);
+    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), kMinIsolationDurationMs);
 
     _circuit_breaker.Reset();
     StartFeedbackThread(&thread_list, &fc_list, 100);
     for (int  i = 0; i < kThreadNum; ++i) {
         void* ret_data = nullptr;
-        EXPECT_EQ(pthread_join(thread_list[i], &ret_data), 0);
+        ASSERT_EQ(pthread_join(thread_list[i], &ret_data), 0);
         FeedbackControl* fc = static_cast<FeedbackControl*>(ret_data);
         EXPECT_FALSE(fc->_healthy);
         EXPECT_LE(fc->_healthy_cnt, kShortWindowSize);
         EXPECT_GT(fc->_unhealthy_cnt, 0);
     }
-    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), kMinIsolationDurationMs * 4);
+    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), kMinIsolationDurationMs * 2);
 
     _circuit_breaker.Reset();
     StartFeedbackThread(&thread_list, &fc_list, 100);
     for (int  i = 0; i < kThreadNum; ++i) {
         void* ret_data = nullptr;
-        EXPECT_EQ(pthread_join(thread_list[i], &ret_data), 0);
+        ASSERT_EQ(pthread_join(thread_list[i], &ret_data), 0);
         FeedbackControl* fc = static_cast<FeedbackControl*>(ret_data);
         EXPECT_FALSE(fc->_healthy);
         EXPECT_LE(fc->_healthy_cnt, kShortWindowSize);
         EXPECT_GT(fc->_unhealthy_cnt, 0);
     }
-    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), kMinIsolationDurationMs * 8);
-}
+    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), kMinIsolationDurationMs * 4);
 
-TEST_F(CircuitBreakerTest, isolation_duration_reset) {
-    std::vector<pthread_t> thread_list;
-    std::vector<std::unique_ptr<FeedbackControl>> fc_list;
     _circuit_breaker.Reset();
-    _circuit_breaker.OnCallEnd(kErrorCodeForFailed, kLatency);
     ::usleep((kMaxIsolationDurationMs + kMinIsolationDurationMs) * 1000);
     StartFeedbackThread(&thread_list, &fc_list, 100);
     for (int  i = 0; i < kThreadNum; ++i) {
         void* ret_data = nullptr;
-        EXPECT_EQ(pthread_join(thread_list[i], &ret_data), 0);
+        ASSERT_EQ(pthread_join(thread_list[i], &ret_data), 0);
         FeedbackControl* fc = static_cast<FeedbackControl*>(ret_data);
         EXPECT_FALSE(fc->_healthy);
         EXPECT_LE(fc->_healthy_cnt, kShortWindowSize);
         EXPECT_GT(fc->_unhealthy_cnt, 0);
     }
     EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), kMinIsolationDurationMs);
+
 }
 
-TEST_F(CircuitBreakerTest, isolation_duration_compute) {
+TEST_F(CircuitBreakerTest, maximum_isolation_duration) {
+    brpc::FLAGS_circuit_breaker_max_isolation_duration_ms =
+        brpc::FLAGS_circuit_breaker_min_isolation_duration_ms + 1;
+    ASSERT_LT(brpc::FLAGS_circuit_breaker_max_isolation_duration_ms,
+              2 * brpc::FLAGS_circuit_breaker_min_isolation_duration_ms);
     std::vector<pthread_t> thread_list;
     std::vector<std::unique_ptr<FeedbackControl>> fc_list;
+
     _circuit_breaker.Reset();
-    ::usleep((kMaxIsolationDurationMs + kMinIsolationDurationMs) * 1000);
     StartFeedbackThread(&thread_list, &fc_list, 100);
     for (int  i = 0; i < kThreadNum; ++i) {
         void* ret_data = nullptr;
-        EXPECT_EQ(pthread_join(thread_list[i], &ret_data), 0);
+        ASSERT_EQ(pthread_join(thread_list[i], &ret_data), 0);
         FeedbackControl* fc = static_cast<FeedbackControl*>(ret_data);
         EXPECT_FALSE(fc->_healthy);
         EXPECT_LE(fc->_healthy_cnt, kShortWindowSize);
         EXPECT_GT(fc->_unhealthy_cnt, 0);
     }
-    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(), 2 * kMinIsolationDurationMs);
+    EXPECT_EQ(_circuit_breaker.isolation_duration_ms(),
+              brpc::FLAGS_circuit_breaker_max_isolation_duration_ms);
 }