Fix time-based circuit breaker statistics after idle window reset

core/src/main/java/dev/failsafe/internal/TimedCircuitStats.java

@@ -27,7 +27,6 @@ class TimedCircuitStats implements CircuitStats {
 
   private final Clock clock;
   private final long bucketSizeMillis;
-  private final long windowSizeMillis;
 
   // Mutable state
   final Bucket[] buckets;
@@ -37,7 +36,6 @@ class TimedCircuitStats implements CircuitStats {
   public TimedCircuitStats(int bucketCount, Duration thresholdingPeriod, Clock clock, CircuitStats oldStats) {
     this.clock = clock;
     this.buckets = new Bucket[bucketCount];
-    windowSizeMillis = thresholdingPeriod.toMillis();
     bucketSizeMillis = thresholdingPeriod.toMillis() / buckets.length;
     for (int i = 0; i < buckets.length; i++)
       buckets[i] = new Bucket();
@@ -197,16 +195,15 @@ synchronized Bucket getCurrentBucket() {
           currentIndex = nextIndex();
           previousBucket = currentBucket;
           currentBucket = buckets[currentIndex];
-          long bucketStartTime = currentBucket.startTimeMillis == -1 ?
-            previousBucket.startTimeMillis + bucketSizeMillis :
-            currentBucket.startTimeMillis + windowSizeMillis;
+          long bucketStartTime = previousBucket.startTimeMillis + bucketSizeMillis;
           summary.remove(currentBucket);
           currentBucket.reset(bucketStartTime);
           bucketsToMove--;
         } while (bucketsToMove > 0);
       } else {
         // Reset all buckets
         reset();
+        currentBucket = buckets[currentIndex];
       }
     }
 

core/src/test/java/dev/failsafe/internal/ClosedStateTest.java

@@ -16,8 +16,11 @@
 package dev.failsafe.internal;
 
 import dev.failsafe.CircuitBreaker;
+import dev.failsafe.internal.TimedCircuitStatsTest.TestClock;
 import org.testng.annotations.Test;
 
+import java.time.Duration;
+
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 
@@ -84,6 +87,25 @@ public void testFailureWithFailureThreshold() {
     assertTrue(breaker.isOpen());
   }
 
+  public void testFailureThresholdAfterIdlePeriod() {
+    CircuitBreakerImpl<Object> breaker = (CircuitBreakerImpl<Object>) CircuitBreaker.builder()
+      .withFailureThreshold(2, Duration.ofSeconds(4))
+      .build();
+    TestClock clock = new TestClock();
+    breaker.state.get().stats = new TimedCircuitStats(4, Duration.ofSeconds(4), clock, null);
+
+    clock.set(1000);
+    breaker.recordSuccess();
+
+    // The failures after an idle period must both count towards the threshold.
+    clock.set(10000);
+    breaker.recordFailure();
+    assertTrue(breaker.isClosed());
+    clock.set(11000);
+    breaker.recordFailure();
+    assertTrue(breaker.isOpen());
+  }
+
   /**
    * Asserts that the circuit is still closed after a single success.
    */

core/src/test/java/dev/failsafe/internal/TimedCircuitStatsTest.java

@@ -18,6 +18,7 @@
 import dev.failsafe.internal.TimedCircuitStats.Bucket;
 import dev.failsafe.internal.TimedCircuitStats.Clock;
 import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 import java.time.Duration;
@@ -152,6 +153,46 @@ public void testMetrics() {
     assertEquals(stats.getExecutionCount(), 0);
   }
 
+  @DataProvider
+  public Object[][] idlePeriods() {
+    return new Object[][] {
+      { 0, true }, { 1, true }, { 2, true }, { 3, true },
+      { 0, false }, { 1, false }, { 2, false }, { 3, false }
+    };
+  }
+
+  @Test(dataProvider = "idlePeriods")
+  public void testExecutionExpiresAfterIdlePeriod(int previousIndex, boolean failure) {
+    stats = new TimedCircuitStats(4, Duration.ofSeconds(4), clock, null);
+    clock.set(previousIndex * 1000);
+    stats.recordSuccess();
+
+    // The first execution after a full window of inactivity must belong to the new current bucket.
+    clock.set(10000);
+    if (failure)
+      stats.recordFailure();
+    else
+      stats.recordSuccess();
+
+    // Keep the execution for the new window, including when the previously current bucket is visited again.
+    for (int seconds = 0; seconds < 4; seconds++) {
+      clock.set(10000 + seconds * 1000);
+      stats.getCurrentBucket();
+      assertEquals(stats.getExecutionCount(), 1);
+      assertEquals(stats.getFailureCount(), failure ? 1 : 0);
+      assertEquals(stats.getSuccessCount(), failure ? 0 : 1);
+      assertEquals(stats.getFailureRate(), failure ? 100 : 0);
+      assertEquals(stats.getSuccessRate(), failure ? 0 : 100);
+    }
+
+    // Expire it exactly when its own window ends.
+    clock.set(14000);
+    stats.getCurrentBucket();
+    assertEquals(stats.getExecutionCount(), 0);
+    assertEquals(stats.getFailureCount(), 0);
+    assertEquals(stats.getSuccessCount(), 0);
+  }
+
   public void testCopyToEqualSizedStats() {
     stats = new TimedCircuitStats(4, Duration.ofSeconds(4), clock, null);
     assertValues(stats, b(0, 0, 0), b(-1, 0, 0), b(-1, 0, 0), b(-1, 0, 0));