fixed retry logic in the case of fail after one more try configuratio…

…n. Also modifies some long standing (and I believe incorrect) behaviour of trying until successful on a timeout exception
rantav · May 20, 2011 · 385303a · 385303a
1 parent 9b777c9
commit 385303a
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 8 deletions.
diff --git a/core/src/main/java/me/prettyprint/cassandra/connection/HConnectionManager.java b/core/src/main/java/me/prettyprint/cassandra/connection/HConnectionManager.java
@@ -227,31 +227,29 @@ public void operateWithFailover(Operation<?> op) throws HectorException {
           // break out on HUnavailableException as well since we can no longer satisfy the CL
           throw he;
         } else if ( he instanceof HectorTransportException) {
-          --retries;
           // client can be null in this situation
           if ( client != null ) {            
             client.close();
           }
           markHostAsDown(pool.getCassandraHost());
           excludeHosts.add(pool.getCassandraHost());
           retryable = true;
-          if ( retries > 0 ) {
-            monitor.incCounter(Counter.RECOVERABLE_TRANSPORT_EXCEPTIONS);
-          }        
+
+          monitor.incCounter(Counter.RECOVERABLE_TRANSPORT_EXCEPTIONS);
+
         } else if (he instanceof HTimedOutException ) {
           // DO NOT drecrement retries, we will be keep retrying on timeouts until it comes back
           // if HLT.checkTimeout(cassandraHost): suspendHost(cassandraHost);          
           doTimeoutCheck(pool.getCassandraHost());
-          if ( hostPools.size() > 1) {
-            retryable = true;
-          }
+
+          retryable = true;
+
           monitor.incCounter(Counter.RECOVERABLE_TIMED_OUT_EXCEPTIONS);
           client.close();
           // TODO timecheck on how long we've been waiting on timeouts here
           // suggestion per user moores on hector-users
         } else if ( he instanceof PoolExhaustedException ) {
           retryable = true;
-          --retries;
           if ( hostPools.size() == 1 ) {
             throw he;
           }
@@ -264,11 +262,13 @@ public void operateWithFailover(Operation<?> op) throws HectorException {
           retryable = false;
         }
         if ( retries <= 0 || retryable == false) throw he;
+
         log.warn("Could not fullfill request on this host {}", client);
         log.warn("Exception: ", he);
         monitor.incCounter(Counter.SKIP_HOST_SUCCESS);
         sleepBetweenHostSkips(op.failoverPolicy);
       } finally {
+        --retries;
         if ( !success ) {
           monitor.incCounter(op.failCounter);
           stopWatch.stop(op.stopWatchTagName + ".fail_");

diff --git a/core/src/test/java/me/prettyprint/cassandra/connection/HConnectionManagerTest.java b/core/src/test/java/me/prettyprint/cassandra/connection/HConnectionManagerTest.java
@@ -2,12 +2,18 @@
 
 import static org.junit.Assert.*;
 
+import org.apache.cassandra.thrift.Cassandra.Client;
 import org.junit.Before;
 import org.junit.Test;
 
 import me.prettyprint.cassandra.BaseEmbededServerSetupTest;
 import me.prettyprint.cassandra.service.CassandraHost;
 import me.prettyprint.cassandra.service.CassandraHostConfigurator;
+import me.prettyprint.cassandra.service.FailoverPolicy;
+import me.prettyprint.cassandra.service.Operation;
+import me.prettyprint.cassandra.service.OperationType;
+import me.prettyprint.hector.api.exceptions.HTimedOutException;
+import me.prettyprint.hector.api.exceptions.HectorException;
 import me.prettyprint.hector.api.exceptions.HectorTransportException;
 
 public class HConnectionManagerTest extends BaseEmbededServerSetupTest {
@@ -56,4 +62,35 @@ public void testSuspendCassandraHost() {
     assertEquals(1,connectionManager.getSuspendedCassandraHosts().size());
     assertTrue(connectionManager.unsuspendCassandraHost(cassandraHost));
   }
+
+  @Test(expected=HTimedOutException.class)
+  public void testTimedOutOperateWithFailover() {
+    setupClient();
+    FailoverPolicy fp = FailoverPolicy.ON_FAIL_TRY_ONE_NEXT_AVAILABLE;
+    connectionManager.operateWithFailover(new TimeoutOp(fp));
+  }
+
+  abstract class StubOp extends Operation<String> {
+
+    StubOp(FailoverPolicy fp) {      
+      this(OperationType.META_READ);
+      failoverPolicy = fp;
+    }
+
+    public StubOp(OperationType operationType) {
+      super(operationType);
+    }    
+  }
+
+  class TimeoutOp extends StubOp {
+
+    TimeoutOp(FailoverPolicy fp) {
+      super(fp);
+    }
+
+    @Override
+    public String execute(Client cassandra) throws HectorException {
+      throw new HTimedOutException("fake timeout");
+    }
+  }
 }