Fix the backoff strategy.

neo4j · Feb 16, 2016 · 53c0589 · 53c0589
1 parent 1ce1ebc
commit 53c0589
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 28 deletions.
diff --git a/...dge/src/main/java/org/neo4j/coreedge/raft/replication/tx/ExpontentialBackoffStrategy.java b/...dge/src/main/java/org/neo4j/coreedge/raft/replication/tx/ExpontentialBackoffStrategy.java
@@ -23,22 +23,31 @@
 
 public class ExpontentialBackoffStrategy implements RetryStrategy
 {
-    private long timeout;
+    private final long initialBackoffTimeMillis;
 
-    public ExpontentialBackoffStrategy( long initialTimeout, TimeUnit timeUnit )
+    public ExpontentialBackoffStrategy( long initialBackoffTime, TimeUnit timeUnit )
     {
-        this.timeout = timeUnit.toMillis( initialTimeout );
+        initialBackoffTimeMillis = timeUnit.toMillis( initialBackoffTime );
     }
 
     @Override
-    public long get()
+    public Timeout newTimeout()
     {
-        return timeout;
-    }
+        return new Timeout()
+        {
+            private long backoffTimeMillis = initialBackoffTimeMillis;
 
-    @Override
-    public void increaseTimeout()
-    {
-        timeout = 2 * timeout;
+            @Override
+            public long getMillis()
+            {
+                return backoffTimeMillis;
+            }
+
+            @Override
+            public void increment()
+            {
+                backoffTimeMillis = backoffTimeMillis * 2;
+            }
+        };
     }
 }
diff --git a/.../main/java/org/neo4j/coreedge/raft/replication/tx/ReplicatedTransactionCommitProcess.java b/.../main/java/org/neo4j/coreedge/raft/replication/tx/ReplicatedTransactionCommitProcess.java
@@ -79,6 +79,7 @@ public long commit( final TransactionToApply tx,
             throw new TransactionFailureException( "Could not create immutable transaction for replication", e );
         }
 
+        RetryStrategy.Timeout timeout = retryStrategy.newTimeout();
         boolean hasNeverReplicated = true;
         boolean interrupted = false;
         try ( CommittingTransaction futureTxId = txFutures.register( operationContext.localOperationId() ) )
@@ -104,8 +105,7 @@ public long commit( final TransactionToApply tx,
 
                 try
                 {
-                    Long txId = futureTxId.waitUntilCommitted( retryStrategy.get(), TimeUnit.MILLISECONDS );
-                    retryStrategy.increaseTimeout();
+                    Long txId = futureTxId.waitUntilCommitted( timeout.getMillis(), TimeUnit.MILLISECONDS );
                     sessionPool.releaseSession( operationContext );
 
                     return txId;
@@ -119,7 +119,8 @@ public long commit( final TransactionToApply tx,
                 catch ( TimeoutException e )
                 {
                     log.info( "Replication of %s timed out after %d %s; retrying.",
-                            operationContext, retryStrategy.get(), TimeUnit.MILLISECONDS );
+                            operationContext, timeout.getMillis(), TimeUnit.MILLISECONDS );
+                    timeout.increment();
                     txRetryMonitor.retry();
                 }
             }

diff --git a/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/replication/tx/RetryStrategy.java b/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/replication/tx/RetryStrategy.java
@@ -21,6 +21,11 @@
 
 public interface RetryStrategy
 {
-    long get();
-    void increaseTimeout();
+    Timeout newTimeout();
+
+    interface Timeout
+    {
+        long getMillis();
+        void increment();
+    }
 }
diff --git a/...-edge/src/test/java/org/neo4j/coreedge/raft/replication/tx/ConstantTimeRetryStrategy.java b/...-edge/src/test/java/org/neo4j/coreedge/raft/replication/tx/ConstantTimeRetryStrategy.java
@@ -23,21 +23,30 @@
 
 public class ConstantTimeRetryStrategy implements RetryStrategy
 {
-    private long timeout;
+    private final Timeout constantTimeout;
 
     public ConstantTimeRetryStrategy( long backoffTime, TimeUnit timeUnit )
     {
-        this.timeout = timeUnit.toMillis( backoffTime );
-    }
+        long backoffTimeMillis = timeUnit.toMillis( backoffTime );
 
-    @Override
-    public long get()
-    {
-        return timeout;
+        constantTimeout = new Timeout()
+        {
+            @Override
+            public long getMillis()
+            {
+                return backoffTimeMillis;
+            }
+
+            @Override
+            public void increment()
+            {
+            }
+        };
     }
 
     @Override
-    public void increaseTimeout()
+    public Timeout newTimeout()
     {
+        return constantTimeout;
     }
 }
diff --git a/...src/test/java/org/neo4j/coreedge/raft/replication/tx/ExpontentialBackoffStrategyTest.java b/...src/test/java/org/neo4j/coreedge/raft/replication/tx/ExpontentialBackoffStrategyTest.java
@@ -34,30 +34,32 @@ public void shouldDoubleEachTime() throws Exception
     {
         // given
         ExpontentialBackoffStrategy strategy = new ExpontentialBackoffStrategy( 1, MILLISECONDS );
+        RetryStrategy.Timeout timeout = strategy.newTimeout();
 
         // when
         for ( int i = 0; i < NUMBER_OF_ACCESSES; i++ )
         {
-            strategy.increaseTimeout();
+            timeout.increment();
         }
 
         // then
-        assertEquals( 2 << NUMBER_OF_ACCESSES - 1, strategy.get() );
+        assertEquals( 2 << NUMBER_OF_ACCESSES - 1, timeout.getMillis() );
     }
 
     @Test
     public void shouldProvidePreviousTimeout() throws Exception
     {
         // given
         ExpontentialBackoffStrategy strategy = new ExpontentialBackoffStrategy( 1, MILLISECONDS );
+        RetryStrategy.Timeout timeout = strategy.newTimeout();
 
         // when
         for ( int i = 0; i <= NUMBER_OF_ACCESSES; i++ )
         {
-            strategy.increaseTimeout();
+            timeout.increment();
         }
 
         // then
-        assertEquals( 2 << NUMBER_OF_ACCESSES, strategy.get() );
+        assertEquals( 2 << NUMBER_OF_ACCESSES, timeout.getMillis() );
     }
-}
+}