Split JedisClusterCommand into multiple methods

No behavior changes, just a refactoring.

Changes:
* Replaces recursion with a for loop
* Extract redirection handling into its own method
* Extract connection-failed handling into its own method

Note that `tryWithRandomNode` is gone, it was never `true` so it and its
code didn't survive the refactoring.

Author: Johan Walles

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

@@ -22,7 +22,7 @@ public JedisClusterCommand(JedisClusterConnectionHandler connectionHandler, int
   public abstract T execute(Jedis connection);
 
   public T run(String key) {
-    return runWithRetries(JedisClusterCRC16.getSlot(key), this.maxAttempts, false, null);
+    return runWithRetries(JedisClusterCRC16.getSlot(key));
   }
 
   public T run(int keyCount, String... keys) {
@@ -42,11 +42,11 @@ public T run(int keyCount, String... keys) {
       }
     }
 
-    return runWithRetries(slot, this.maxAttempts, false, null);
+    return runWithRetries(slot);
   }
 
   public T runBinary(byte[] key) {
-    return runWithRetries(JedisClusterCRC16.getSlot(key), this.maxAttempts, false, null);
+    return runWithRetries(JedisClusterCRC16.getSlot(key));
   }
 
   public T runBinary(int keyCount, byte[]... keys) {
@@ -66,7 +66,7 @@ public T runBinary(int keyCount, byte[]... keys) {
       }
     }
 
-    return runWithRetries(slot, this.maxAttempts, false, null);
+    return runWithRetries(slot);
   }
 
   public T runWithAnyNode() {
@@ -79,62 +79,87 @@ public T runWithAnyNode() {
     }
   }
 
-  private T runWithRetries(final int slot, int attempts, boolean tryRandomNode, JedisRedirectionException redirect) {
-    if (attempts <= 0) {
-      throw new JedisClusterMaxAttemptsException("No more cluster attempts left.");
-    }
+  private interface ConnectionGetter {
+    Jedis getConnection();
+  }
 
-    Jedis connection = null;
-    try {
+  private T runWithRetries(final int slot) {
+    ConnectionGetter connectionGetter = new ConnectionGetter() {
+      @Override
+      public Jedis getConnection() {
+        return connectionHandler.getConnectionFromSlot(slot);
+      }
+    };
 
-      if (redirect != null) {
-        connection = this.connectionHandler.getConnectionFromNode(redirect.getTargetNode());
-        if (redirect instanceof JedisAskDataException) {
-          // TODO: Pipeline asking with the original command to make it faster....
-          connection.asking();
-        }
-      } else {
-        if (tryRandomNode) {
-          connection = connectionHandler.getConnection();
+    // If we got one redirection, stick with that and don't try anything else
+    ConnectionGetter redirected = null;
+
+    for (int currentAttempt = 0; currentAttempt < this.maxAttempts; currentAttempt++) {
+      Jedis connection = null;
+      try {
+        if (redirected != null) {
+          connection = redirected.getConnection();
         } else {
-          connection = connectionHandler.getConnectionFromSlot(slot);
+          connection = connectionGetter.getConnection();
+        }
+        return execute(connection);
+      } catch (JedisNoReachableClusterNodeException e) {
+        throw e;
+      } catch (JedisConnectionException e) {
+        connectionGetter = handleConnectionProblem(slot, currentAttempt);
+      } catch (JedisRedirectionException e) {
+        redirected = handleRedirection(connection, e);
+      } finally {
+        if (connection != null) {
+          releaseConnection(connection);
         }
       }
+    }
 
-      return execute(connection);
+    throw new JedisClusterMaxAttemptsException("No more cluster attempts left.");
+  }
 
-    } catch (JedisNoReachableClusterNodeException jnrcne) {
-      throw jnrcne;
-    } catch (JedisConnectionException jce) {
-      // release current connection before recursion
-      releaseConnection(connection);
-      connection = null;
-
-      if (attempts <= 1) {
-        //We need this because if node is not reachable anymore - we need to finally initiate slots
-        //renewing, or we can stuck with cluster state without one node in opposite case.
-        //But now if maxAttempts = [1 or 2] we will do it too often.
-        //TODO make tracking of successful/unsuccessful operations for node - do renewing only
-        //if there were no successful responses from this node last few seconds
-        this.connectionHandler.renewSlotCache();
-      }
+  private ConnectionGetter handleConnectionProblem(final int slot, int currentAttempt) {
+    int attemptsLeft = (maxAttempts - currentAttempt) - 1;
+    if (attemptsLeft <= 1) {
+      //We need this because if node is not reachable anymore - we need to finally initiate slots
+      //renewing, or we can stuck with cluster state without one node in opposite case.
+      //But now if maxAttempts = [1 or 2] we will do it too often.
+      //TODO make tracking of successful/unsuccessful operations for node - do renewing only
+      //if there were no successful responses from this node last few seconds
+      this.connectionHandler.renewSlotCache();
+    }
 
-      return runWithRetries(slot, attempts - 1, tryRandomNode, redirect);
-    } catch (JedisRedirectionException jre) {
-      // if MOVED redirection occurred,
-      if (jre instanceof JedisMovedDataException) {
-        // it rebuilds cluster's slot cache recommended by Redis cluster specification
-        this.connectionHandler.renewSlotCache(connection);
+    return new ConnectionGetter() {
+      @Override
+      public Jedis getConnection() {
+        return connectionHandler.getConnectionFromSlot(slot);
       }
+    };
+  }
 
-      // release current connection before recursion
-      releaseConnection(connection);
-      connection = null;
-
-      return runWithRetries(slot, attempts - 1, false, jre);
-    } finally {
-      releaseConnection(connection);
+  private ConnectionGetter handleRedirection(Jedis connection, final JedisRedirectionException jre) {
+    // if MOVED redirection occurred,
+    if (jre instanceof JedisMovedDataException) {
+      // it rebuilds cluster's slot cache recommended by Redis cluster specification
+      this.connectionHandler.renewSlotCache(connection);
     }
+
+    // release current connection before iteration
+    releaseConnection(connection);
+
+    return new ConnectionGetter() {
+      @Override
+      public Jedis getConnection() {
+        Jedis connection = connectionHandler.getConnectionFromNode(jre.getTargetNode());
+        if (jre instanceof JedisAskDataException) {
+          // TODO: Pipeline asking with the original command to make it faster....
+          connection.asking();
+        }
+
+        return connection;
+      }
+    };
   }
 
   private void releaseConnection(Jedis connection) {