[automatic failover] Improve failover resilience by evaluating circuit breaker before retries

formatter-pom.xml

@@ -20,7 +20,7 @@
 			<plugin>
 				<groupId>net.revelc.code.formatter</groupId>
 				<artifactId>formatter-maven-plugin</artifactId>
-				<version>2.23.0</version>
+				<version>2.16.0</version>
 				<configuration>
 					<configFile>${project.basedir}/hbase-formatter.xml</configFile>
 				</configuration>

src/main/java/redis/clients/jedis/mcf/CircuitBreakerCommandExecutor.java

@@ -35,8 +35,8 @@ public <T> T executeCommand(CommandObject<T> commandObject) {
     DecorateSupplier<T> supplier = Decorators
         .ofSupplier(() -> this.handleExecuteCommand(commandObject, cluster));
 
-    supplier.withRetry(cluster.getRetry());
     supplier.withCircuitBreaker(cluster.getCircuitBreaker());
+    supplier.withRetry(cluster.getRetry());
     supplier.withFallback(provider.getFallbackExceptionList(),
       e -> this.handleClusterFailover(commandObject, cluster.getCircuitBreaker()));
 

src/test/java/redis/clients/jedis/failover/FailoverIntegrationTest.java

@@ -272,4 +272,62 @@ public void testManualFailoverInflightCommandsWithErrorsPropagateError() throws
     assertThat(getNodeId(failoverClient.info("server")), equalTo(JEDIS2_ID));
   }
 
-}
+  /**
+   * Tests that the CircuitBreaker counts each command error separately, and not just after all
+   * retries are exhausted. This ensures that the circuit breaker opens based on the actual number
+   * of send commands with failures, and not based on the number of logical operations.
+   */
+  @Test
+  public void testCircuitBreakerCountsEachConnectionErrorSeparately() throws IOException {
+    MultiClusterClientConfig failoverConfig = new MultiClusterClientConfig.Builder(
+        getClusterConfigs(
+          DefaultJedisClientConfig.builder()
+              .socketTimeoutMillis(RecommendedSettings.DEFAULT_TIMEOUT_MS)
+              .connectionTimeoutMillis(RecommendedSettings.DEFAULT_TIMEOUT_MS).build(),
+          endpoint1, endpoint2)).retryMaxAttempts(2).retryWaitDuration(1)
+              .circuitBreakerSlidingWindowType(COUNT_BASED).circuitBreakerSlidingWindowSize(3)
+              .circuitBreakerFailureRateThreshold(50) // 50% failure
+                                                      // rate threshold
+              .circuitBreakerSlidingWindowMinCalls(3).build();
+
+    MultiClusterPooledConnectionProvider provider = new MultiClusterPooledConnectionProvider(
+        failoverConfig);
+    try (UnifiedJedis client = new UnifiedJedis(provider)) {
+      // Verify initial connection to first endpoint
+      assertThat(getNodeId(client.info("server")), equalTo(JEDIS1_ID));
+
+      // Disable first endpoint
+      redisProxy1.disable();
+
+      // First command should fail and OPEN the circuit breaker immediately
+      //
+      // If CB is applied after retries:
+      // - It would take 2 commands to OPEN CB (error is propagated for both commands)
+      // - Failover to the next Endpoint happens on the 3rd command
+      //
+      // If CB is applied before retries:
+      // - It should open after just 1 command with retries
+      // - CB is OPEN after the 2nd retry of the first command
+      // - Failover to the next Endpoint happens on the 2nd command
+      //
+      // This test verifies the second case by checking that:
+      // 1. CB opens after the first command (with retries)
+      // 2. The second command is routed to the second endpoint
+      // Command 1
+      assertThrows(JedisConnectionException.class, () -> client.info("server"));
+
+      // Circuit breaker should be open after just one command with retries
+      assertThat(provider.getCluster(1).getCircuitBreaker().getState(),
+        equalTo(CircuitBreaker.State.OPEN));
+
+      // Next command should be routed to the second endpoint
+      // Command 2
+      assertThat(getNodeId(client.info("server")), equalTo(JEDIS2_ID));
+
+      // Command 3
+      assertThat(getNodeId(client.info("server")), equalTo(JEDIS2_ID));
+
+    }
+  }
+
+}