Use circuit breaker fallback exception list (#3664)

* Use circuit breaker fallback exception list

* remove starting file

* restore condition

* format

* Reformat codes

* Failover from auth error

* rename

* Reformat codes

* Reformat variables

* Added doc and renamed

* Undo public changes

* Spellcheck wordlist

* Removed checking of values in config builder

as builder variables now have proper default values.

.github/wordlist.txt

@@ -7,6 +7,7 @@ BitPosParams
 BuilderFactory
 CFCommands
 CMSCommands
+CallNotPermittedException
 CircuitBreaker
 ClientKillParams
 ClusterNode
@@ -40,6 +41,7 @@ Jaeger
 Javadocs
 Jedis
 JedisCluster
+JedisConnectionException
 JedisPool
 JedisPooled
 JedisShardInfo
@@ -88,6 +90,7 @@ StatusCode
 StreamEntryID
 TCP
 TOPKCommands
+Throwable
 TimeSeriesCommands
 URI
 UnblockType

docs/failover.md

@@ -99,8 +99,8 @@ Jedis uses the following retry settings:
 | Max retry attempts               | 3                          | Maximum number of retry attempts (including the initial call)                                                                                                                                                   |
 | Retry wait duration              | 500 ms                     | Number of milliseconds to wait between retry attempts                                                                                                                                                           |
 | Wait duration backoff multiplier | 2                          | Exponential backoff factor multiplied against wait duration between retries. For example, with a wait duration of 1 second and a multiplier of 2, the retries would occur after 1s, 2s, 4s, 8s, 16s, and so on. |
-| Retry included exception list    | `JedisConnectionException` | A list of `Throwable` classes that count as failures and should be retried.                                                                                                                                     |
-| Retry ignored exception list     | Empty list                 | A list of `Throwable` classes to explicitly ignore for the purposes of retry.                                                                                                                                   |
+| Retry included exception list    | [JedisConnectionException] | A list of Throwable classes that count as failures and should be retried.                                                                                                                                       |
+| Retry ignored exception list     | null                       | A list of Throwable classes to explicitly ignore for the purposes of retry.                                                                                                                                     |
 
 To disable retry, set `maxRetryAttempts` to 1.
 
@@ -116,8 +116,16 @@ Jedis uses the following circuit breaker settings:
 | Failure rate threshold                  | `50.0f`                    | Percentage of calls within the sliding window that must fail before the circuit breaker transitions to the `OPEN` state.                                                      |
 | Slow call duration threshold            | 60000 ms                   | Duration threshold above which calls are classified as slow and added to the sliding window.                                                                                  |
 | Slow call rate threshold                | `100.0f`                   | Percentage of calls within the sliding window that exceed the slow call duration threshold before circuit breaker transitions to the `OPEN` state.                            |
-| Circuit breaker included exception list | `JedisConnectionException` | A list of `Throwable` classes that count as failures and add to the failure rate.                                                                                             |
-| Circuit breaker ignored exception list  | Empty list                 | A list of `Throwable` classes to explicitly ignore for failure rate calculations.                                                                                             |                                                                                                               |
+| Circuit breaker included exception list | [JedisConnectionException] | A list of Throwable classes that count as failures and add to the failure rate.                                                                                               |
+| Circuit breaker ignored exception list  | null                       | A list of Throwable classes to explicitly ignore for failure rate calculations.                                                                                               |                                                                                                               |
+
+### Fallback configuration
+
+Jedis uses the following fallback settings:
+
+| Setting                 | Default value                                         | Description                                        |
+|-------------------------|-------------------------------------------------------|----------------------------------------------------|
+| Fallback exception list | [CallNotPermittedException, JedisConnectionException] | A list of Throwable classes that trigger fallback. |
 
 ### Failover callbacks
 

src/main/java/redis/clients/jedis/MultiClusterClientConfig.java

@@ -1,13 +1,15 @@
 package redis.clients.jedis;
 
+import io.github.resilience4j.circuitbreaker.CallNotPermittedException;
 import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig.SlidingWindowType;
-import redis.clients.jedis.exceptions.JedisConnectionException;
-import redis.clients.jedis.exceptions.JedisValidationException;
 
 import java.time.Duration;
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
+import redis.clients.jedis.exceptions.JedisConnectionException;
+import redis.clients.jedis.exceptions.JedisValidationException;
+
 
 /**
  * @author Allen Terleto (aterleto)
@@ -22,20 +24,24 @@
  * not passed through to Jedis users.
  * <p>
  */
+// TODO: move
 public final class MultiClusterClientConfig {
 
     private static final int RETRY_MAX_ATTEMPTS_DEFAULT = 3;
     private static final int RETRY_WAIT_DURATION_DEFAULT = 500;  // measured in milliseconds
     private static final int RETRY_WAIT_DURATION_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT = 2;
-    private static final Class RETRY_INCLUDED_EXCEPTIONS_DEFAULT = JedisConnectionException.class;
+    private static final List<Class> RETRY_INCLUDED_EXCEPTIONS_DEFAULT = Arrays.asList(JedisConnectionException.class);
 
     private static final float CIRCUIT_BREAKER_FAILURE_RATE_THRESHOLD_DEFAULT = 50.0f; // measured as percentage
     private static final int CIRCUIT_BREAKER_SLIDING_WINDOW_MIN_CALLS_DEFAULT = 100;
     private static final SlidingWindowType CIRCUIT_BREAKER_SLIDING_WINDOW_TYPE_DEFAULT = SlidingWindowType.COUNT_BASED;
     private static final int CIRCUIT_BREAKER_SLIDING_WINDOW_SIZE_DEFAULT = 100;
     private static final int CIRCUIT_BREAKER_SLOW_CALL_DURATION_THRESHOLD_DEFAULT = 60000; // measured in milliseconds
     private static final float CIRCUIT_BREAKER_SLOW_CALL_RATE_THRESHOLD_DEFAULT = 100.0f; // measured as percentage
-    private static final Class CIRCUIT_BREAKER_INCLUDED_EXCEPTIONS_DEFAULT = JedisConnectionException.class;
+    private static final List<Class> CIRCUIT_BREAKER_INCLUDED_EXCEPTIONS_DEFAULT = Arrays.asList(JedisConnectionException.class);
+
+    private static final List<Class<? extends Throwable>> FALLBACK_EXCEPTIONS_DEFAULT =
+        Arrays.asList(CallNotPermittedException.class, JedisConnectionException.class);
 
     private final ClusterConfig[] clusterConfigs;
 
@@ -99,6 +105,7 @@ public final class MultiClusterClientConfig {
      * failure nor success, even if the exceptions is part of recordExceptions */
     private List<Class> circuitBreakerIgnoreExceptionList;
 
+    private List<Class<? extends Throwable>> fallbackExceptionList;
 
     public MultiClusterClientConfig(ClusterConfig[] clusterConfigs) {
         this.clusterConfigs = clusterConfigs;
@@ -160,6 +167,10 @@ public SlidingWindowType getCircuitBreakerSlidingWindowType() {
         return circuitBreakerSlidingWindowType;
     }
 
+    public List<Class<? extends Throwable>> getFallbackExceptionList() {
+        return fallbackExceptionList;
+    }
+
     public static class ClusterConfig {
 
         private int priority;
@@ -195,18 +206,18 @@ public static class Builder {
         private int retryMaxAttempts = RETRY_MAX_ATTEMPTS_DEFAULT;
         private int retryWaitDuration = RETRY_WAIT_DURATION_DEFAULT;
         private int retryWaitDurationExponentialBackoffMultiplier = RETRY_WAIT_DURATION_EXPONENTIAL_BACKOFF_MULTIPLIER_DEFAULT;
-        private List<Class> retryIncludedExceptionList;
-        private List<Class> retryIgnoreExceptionList;
+        private List<Class> retryIncludedExceptionList = RETRY_INCLUDED_EXCEPTIONS_DEFAULT;
+        private List<Class> retryIgnoreExceptionList = null;
 
         private float circuitBreakerFailureRateThreshold = CIRCUIT_BREAKER_FAILURE_RATE_THRESHOLD_DEFAULT;
         private int circuitBreakerSlidingWindowMinCalls = CIRCUIT_BREAKER_SLIDING_WINDOW_MIN_CALLS_DEFAULT;
         private SlidingWindowType circuitBreakerSlidingWindowType = CIRCUIT_BREAKER_SLIDING_WINDOW_TYPE_DEFAULT;
         private int circuitBreakerSlidingWindowSize = CIRCUIT_BREAKER_SLIDING_WINDOW_SIZE_DEFAULT;
         private int circuitBreakerSlowCallDurationThreshold = CIRCUIT_BREAKER_SLOW_CALL_DURATION_THRESHOLD_DEFAULT;
         private float circuitBreakerSlowCallRateThreshold = CIRCUIT_BREAKER_SLOW_CALL_RATE_THRESHOLD_DEFAULT;
-        private List<Class> circuitBreakerIncludedExceptionList;
-        private List<Class> circuitBreakerIgnoreExceptionList;
-        private List<Class<? extends Throwable>> circuitBreakerFallbackExceptionList;
+        private List<Class> circuitBreakerIncludedExceptionList = CIRCUIT_BREAKER_INCLUDED_EXCEPTIONS_DEFAULT;
+        private List<Class> circuitBreakerIgnoreExceptionList = null;
+        private List<Class<? extends Throwable>> fallbackExceptionList = FALLBACK_EXCEPTIONS_DEFAULT;
 
         public Builder(ClusterConfig[] clusterConfigs) {
 
@@ -219,6 +230,10 @@ public Builder(ClusterConfig[] clusterConfigs) {
             this.clusterConfigs = clusterConfigs;
         }
 
+        public Builder(List<ClusterConfig> clusterConfigs) {
+            this(clusterConfigs.toArray(new ClusterConfig[0]));
+        }
+
         public Builder retryMaxAttempts(int retryMaxAttempts) {
             this.retryMaxAttempts = retryMaxAttempts;
             return this;
@@ -284,8 +299,16 @@ public Builder circuitBreakerIgnoreExceptionList(List<Class> circuitBreakerIgnor
             return this;
         }
 
+        /**
+         * @deprecated Use {@link #fallbackExceptionList(java.util.List)}.
+         */
+        @Deprecated
         public Builder circuitBreakerFallbackExceptionList(List<Class<? extends Throwable>> circuitBreakerFallbackExceptionList) {
-            this.circuitBreakerFallbackExceptionList = circuitBreakerFallbackExceptionList;
+            return fallbackExceptionList(circuitBreakerFallbackExceptionList);
+        }
+
+        public Builder fallbackExceptionList(List<Class<? extends Throwable>> fallbackExceptionList) {
+            this.fallbackExceptionList = fallbackExceptionList;
             return this;
         }
 
@@ -296,16 +319,9 @@ public MultiClusterClientConfig build() {
             config.retryWaitDuration = Duration.ofMillis(this.retryWaitDuration);
             config.retryWaitDurationExponentialBackoffMultiplier = this.retryWaitDurationExponentialBackoffMultiplier;
 
-            if (this.retryIncludedExceptionList != null && !retryIncludedExceptionList.isEmpty())
-                config.retryIncludedExceptionList = this.retryIncludedExceptionList;
-
-            else {
-                config.retryIncludedExceptionList = new ArrayList<>();
-                config.retryIncludedExceptionList.add(RETRY_INCLUDED_EXCEPTIONS_DEFAULT);
-            }
+            config.retryIncludedExceptionList = this.retryIncludedExceptionList;
 
-            if (this.retryIgnoreExceptionList != null && !retryIgnoreExceptionList.isEmpty())
-                config.retryIgnoreExceptionList = this.retryIgnoreExceptionList;
+            config.retryIgnoreExceptionList = this.retryIgnoreExceptionList;
 
             config.circuitBreakerFailureRateThreshold = this.circuitBreakerFailureRateThreshold;
             config.circuitBreakerSlidingWindowMinCalls = this.circuitBreakerSlidingWindowMinCalls;
@@ -314,16 +330,11 @@ public MultiClusterClientConfig build() {
             config.circuitBreakerSlowCallDurationThreshold = Duration.ofMillis(this.circuitBreakerSlowCallDurationThreshold);
             config.circuitBreakerSlowCallRateThreshold = this.circuitBreakerSlowCallRateThreshold;
 
-            if (this.circuitBreakerIncludedExceptionList != null && !circuitBreakerIncludedExceptionList.isEmpty())
-                config.circuitBreakerIncludedExceptionList = this.circuitBreakerIncludedExceptionList;
+            config.circuitBreakerIncludedExceptionList = this.circuitBreakerIncludedExceptionList;
 
-            else {
-                config.circuitBreakerIncludedExceptionList = new ArrayList<>();
-                config.circuitBreakerIncludedExceptionList.add(CIRCUIT_BREAKER_INCLUDED_EXCEPTIONS_DEFAULT);
-            }
+            config.circuitBreakerIgnoreExceptionList = this.circuitBreakerIgnoreExceptionList;
 
-            if (this.circuitBreakerIgnoreExceptionList != null && !circuitBreakerIgnoreExceptionList.isEmpty())
-                config.circuitBreakerIgnoreExceptionList = this.circuitBreakerIgnoreExceptionList;
+            config.fallbackExceptionList = this.fallbackExceptionList;
 
             return config;
         }

src/main/java/redis/clients/jedis/Protocol.java

@@ -54,6 +54,7 @@ public final class Protocol {
   private static final String CLUSTERDOWN_PREFIX = "CLUSTERDOWN ";
   private static final String BUSY_PREFIX = "BUSY ";
   private static final String NOSCRIPT_PREFIX = "NOSCRIPT ";
+  private static final String NOAUTH_PREFIX = "NOAUTH";
   private static final String WRONGPASS_PREFIX = "WRONGPASS";
   private static final String NOPERM_PREFIX = "NOPERM";
 
@@ -97,9 +98,9 @@ private static void processError(final RedisInputStream is) {
       throw new JedisBusyException(message);
     } else if (message.startsWith(NOSCRIPT_PREFIX)) {
       throw new JedisNoScriptException(message);
-    } else if (message.startsWith(WRONGPASS_PREFIX)) {
-      throw new JedisAccessControlException(message);
-    } else if (message.startsWith(NOPERM_PREFIX)) {
+    } else if (message.startsWith(NOAUTH_PREFIX)
+        || message.startsWith(WRONGPASS_PREFIX)
+        || message.startsWith(NOPERM_PREFIX)) {
       throw new JedisAccessControlException(message);
     }
     throw new JedisDataException(message);

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

@@ -32,8 +32,8 @@ public <T> T executeCommand(CommandObject<T> commandObject) {
 
         supplier.withRetry(cluster.getRetry());
         supplier.withCircuitBreaker(cluster.getCircuitBreaker());
-        supplier.withFallback(defaultCircuitBreakerFallbackException,
-                              e -> this.handleClusterFailover(commandObject, cluster.getCircuitBreaker()));
+        supplier.withFallback(provider.getFallbackExceptionList(),
+                e -> this.handleClusterFailover(commandObject, cluster.getCircuitBreaker()));
 
         return supplier.decorate().get();
     }

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

@@ -1,11 +1,6 @@
 package redis.clients.jedis.mcf;
 
-import io.github.resilience4j.circuitbreaker.CallNotPermittedException;
 import io.github.resilience4j.circuitbreaker.CircuitBreaker;
-
-import java.util.Arrays;
-import java.util.List;
-
 import redis.clients.jedis.exceptions.JedisConnectionException;
 import redis.clients.jedis.providers.MultiClusterPooledConnectionProvider;
 import redis.clients.jedis.util.IOUtils;
@@ -21,9 +16,6 @@
  */
 public class CircuitBreakerFailoverBase implements AutoCloseable {
 
-    protected final static List<Class<? extends Throwable>> defaultCircuitBreakerFallbackException =
-                                                          Arrays.asList(CallNotPermittedException.class);
-
     protected final MultiClusterPooledConnectionProvider provider;
 
     public CircuitBreakerFailoverBase(MultiClusterPooledConnectionProvider provider) {

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

@@ -26,8 +26,8 @@ public Connection getConnection() {
 
         supplier.withRetry(cluster.getRetry());
         supplier.withCircuitBreaker(cluster.getCircuitBreaker());
-        supplier.withFallback(defaultCircuitBreakerFallbackException,
-                              e -> this.handleClusterFailover(cluster.getCircuitBreaker()));
+        supplier.withFallback(provider.getFallbackExceptionList(),
+                e -> this.handleClusterFailover(cluster.getCircuitBreaker()));
 
         return supplier.decorate().get();
     }

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

@@ -22,12 +22,13 @@ public class MultiClusterPipeline extends PipelineBase implements Closeable {
 
   public MultiClusterPipeline(MultiClusterPooledConnectionProvider pooledProvider) {
     super(new CommandObjects());
-    try (Connection connection = pooledProvider.getConnection()) { // we don't need a healthy connection now
+
+    this.failoverProvider = new CircuitBreakerFailoverConnectionProvider(pooledProvider);
+
+    try (Connection connection = failoverProvider.getConnection()) {
       RedisProtocol proto = connection.getRedisProtocol();
       if (proto != null) this.commandObjects.setProtocol(proto);
     }
-
-    this.failoverProvider = new CircuitBreakerFailoverConnectionProvider(pooledProvider);
   }
 
   @Override

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

@@ -26,7 +26,7 @@ public class MultiClusterTransaction extends TransactionBase {
 
   private static final Builder<?> NO_OP_BUILDER = BuilderFactory.RAW_OBJECT;
 
-  private final CircuitBreakerFailoverConnectionProvider provider;
+  private final CircuitBreakerFailoverConnectionProvider failoverProvider;
   private final AtomicInteger extraCommandCount = new AtomicInteger();
   private final Queue<KeyValue<CommandArguments, Response<?>>> commands = new LinkedList<>();
 
@@ -50,13 +50,13 @@ public MultiClusterTransaction(MultiClusterPooledConnectionProvider provider) {
    * @param doMulti {@code false} should be set to enable manual WATCH, UNWATCH and MULTI
    */
   public MultiClusterTransaction(MultiClusterPooledConnectionProvider provider, boolean doMulti) {
-    try (Connection connection = provider.getConnection()) { // we don't need a healthy connection now
+    this.failoverProvider = new CircuitBreakerFailoverConnectionProvider(provider);
+
+    try (Connection connection = failoverProvider.getConnection()) {
       RedisProtocol proto = connection.getRedisProtocol();
       if (proto != null) this.commandObjects.setProtocol(proto);
     }
 
-    this.provider = new CircuitBreakerFailoverConnectionProvider(provider);
-
     if (doMulti) multi();
   }
 
@@ -129,7 +129,7 @@ public final List<Object> exec() {
       throw new IllegalStateException("EXEC without MULTI");
     }
 
-    try (Connection connection = provider.getConnection()) {
+    try (Connection connection = failoverProvider.getConnection()) {
 
       commands.forEach((command) -> connection.sendCommand(command.getKey()));
       // following connection.getMany(int) flushes anyway, so no flush here.
@@ -174,7 +174,7 @@ public final String discard() {
       throw new IllegalStateException("DISCARD without MULTI");
     }
 
-    try (Connection connection = provider.getConnection()) {
+    try (Connection connection = failoverProvider.getConnection()) {
 
       commands.forEach((command) -> connection.sendCommand(command.getKey()));
       // following connection.getMany(int) flushes anyway, so no flush here.