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 <[email protected]> Authored: Wed Sep 19 14:56:01 2018 +0530 Committer: Lokesh Jain <[email protected]> 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); + } }
