You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ratis.apache.org by lj...@apache.org on 2018/09/19 09:26:34 UTC

incubator-ratis git commit: RATIS-325. RetryPolicies should not import com.google.common.annotations.VisibleForTesting. Contributed by Tsz Wo Nicholas Sze.

Repository: incubator-ratis
Updated Branches:
  refs/heads/master 25d4a5915 -> 974919e5e


RATIS-325. RetryPolicies should not import com.google.common.annotations.VisibleForTesting. Contributed by Tsz Wo Nicholas Sze.


Project: http://git-wip-us.apache.org/repos/asf/incubator-ratis/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ratis/commit/974919e5
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ratis/tree/974919e5
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ratis/diff/974919e5

Branch: refs/heads/master
Commit: 974919e5e099b80fd85a4ba1bb745db369519b75
Parents: 25d4a59
Author: Lokesh Jain <lj...@apache.org>
Authored: Wed Sep 19 14:56:01 2018 +0530
Committer: Lokesh Jain <lj...@apache.org>
Committed: Wed Sep 19 14:56:01 2018 +0530

----------------------------------------------------------------------
 .../org/apache/ratis/client/RaftClient.java     |  4 +-
 .../org/apache/ratis/retry/RetryPolicies.java   | 85 +++++++++-----------
 .../org/apache/ratis/retry/RetryPolicy.java     |  6 +-
 3 files changed, 42 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/974919e5/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java
----------------------------------------------------------------------
diff --git a/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java b/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java
index ea8b1a2..725b456 100644
--- a/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java
+++ b/ratis-client/src/main/java/org/apache/ratis/client/RaftClient.java
@@ -113,7 +113,7 @@ public interface RaftClient extends Closeable {
     private RaftPeerId leaderId;
     private RaftProperties properties;
     private Parameters parameters;
-    private RetryPolicy retryPolicy;
+    private RetryPolicy retryPolicy = RetryPolicies.retryForeverNoSleep();
 
     private Builder() {}
 
@@ -129,8 +129,6 @@ public interface RaftClient extends Closeable {
           clientRpc = factory.newRaftClientRpc(clientId, properties);
         }
       }
-      retryPolicy =
-          retryPolicy == null ? RetryPolicies.RETRY_FOREVER : retryPolicy;
       return ClientImplUtils.newRaftClient(clientId,
           Objects.requireNonNull(group, "The 'group' field is not initialized."),
           leaderId,

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/974919e5/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicies.java
----------------------------------------------------------------------
diff --git a/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicies.java b/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicies.java
index 4505c8d..1f2df97 100644
--- a/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicies.java
+++ b/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicies.java
@@ -17,39 +17,61 @@
  */
 package org.apache.ratis.retry;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.apache.ratis.util.TimeDuration;
 
-import java.util.concurrent.TimeUnit;
-
 /**
  * A collection of {@link RetryPolicy} implementations
  */
-public class RetryPolicies {
+public interface RetryPolicies {
   /**
-   * Keep retrying forever.
+   * Keep retrying forever with zero sleep.
    */
-  public static final RetryPolicy RETRY_FOREVER = new RetryForever();
+  static RetryPolicy retryForeverNoSleep() {
+    return Constants.RETRY_FOREVER_NO_SLEEP;
+  }
+
+  static RetryPolicy noRetry() {
+    return Constants.NO_RETRY;
+  }
 
   /**
    * Keep trying a limited number of times, waiting a fixed time between attempts,
    * and then fail by re-throwing the exception.
    */
-  public static final RetryPolicy retryUpToMaximumCountWithFixedSleep(
-      int maxRetries, TimeDuration sleepTime) {
-    return new RetryUpToMaximumCountWithFixedSleep(maxRetries, sleepTime);
+  static RetryPolicy retryUpToMaximumCountWithFixedSleep(int maxRetries, TimeDuration sleepTime) {
+    return new RetryLimited(maxRetries, sleepTime);
   }
 
+  class Constants {
+    private static final RetryForeverNoSleep RETRY_FOREVER_NO_SLEEP = new RetryForeverNoSleep();
+    private static final NoRetry NO_RETRY = new NoRetry();
+  }
+
+  class RetryForeverNoSleep implements RetryPolicy {
+    private RetryForeverNoSleep() {}
 
-  static class RetryForever implements RetryPolicy {
     @Override
     public boolean shouldRetry(int retryCount) {
       return true;
     }
 
     @Override
-    public TimeDuration getSleepTime() {
-      return TimeDuration.valueOf(0, TimeUnit.MILLISECONDS);
+    public String toString() {
+      return getClass().getSimpleName();
+    }
+  }
+
+  class NoRetry implements RetryPolicy {
+    private NoRetry() {}
+
+    @Override
+    public boolean shouldRetry(int retryCount) {
+      return false;
+    }
+
+    @Override
+    public String toString() {
+      return getClass().getSimpleName();
     }
   }
 
@@ -61,7 +83,7 @@ public class RetryPolicies {
    * The object of the subclasses should be immutable;
    * otherwise, the subclass must override hashCode(), equals(..) and toString().
    */
-  static abstract class RetryLimited implements RetryPolicy {
+  class RetryLimited implements RetryPolicy {
     private final int maxRetries;
     private final TimeDuration sleepTime;
 
@@ -91,36 +113,7 @@ public class RetryPolicies {
 
     @Override
     public boolean shouldRetry(int retryCount) {
-      if (retryCount >= maxRetries) {
-        return false;
-      } else {
-        return true;
-      }
-    }
-
-    protected String getReason() {
-      return constructReasonString(maxRetries);
-    }
-
-    @VisibleForTesting
-    public static String constructReasonString(int retries) {
-      return "retries get failed due to exceeded maximum allowed retries " +
-          "number: " + retries;
-    }
-
-    @Override
-    public int hashCode() {
-      return toString().hashCode();
-    }
-
-    @Override
-    public boolean equals(final Object that) {
-      if (this == that) {
-        return true;
-      } else if (that == null || this.getClass() != that.getClass()) {
-        return false;
-      }
-      return this.toString().equals(that.toString());
+      return retryCount < maxRetries;
     }
 
     @Override
@@ -132,10 +125,4 @@ public class RetryPolicies {
       return myString;
     }
   }
-
-  static class RetryUpToMaximumCountWithFixedSleep extends RetryLimited {
-    public RetryUpToMaximumCountWithFixedSleep(int maxRetries, TimeDuration sleepTime) {
-      super(maxRetries, sleepTime);
-    }
-  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/974919e5/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicy.java
----------------------------------------------------------------------
diff --git a/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicy.java b/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicy.java
index fe4ee80..3de972a 100644
--- a/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicy.java
+++ b/ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicy.java
@@ -19,6 +19,8 @@ package org.apache.ratis.retry;
 
 import org.apache.ratis.util.TimeDuration;
 
+import java.util.concurrent.TimeUnit;
+
 /**
  * Policy abstract for retrying.
  */
@@ -36,5 +38,7 @@ public interface RetryPolicy {
   /**
    * Returns the time duration for sleep in between the retries.
    */
-  TimeDuration getSleepTime();
+  default TimeDuration getSleepTime() {
+    return TimeDuration.valueOf(0, TimeUnit.MILLISECONDS);
+  }
 }