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 2020/01/31 17:21:27 UTC
[incubator-ratis] branch master updated: RATIS-799. Add a new retry
policy Exception dependent retry policy. Contributed by Bharat Viswanadham.
This is an automated email from the ASF dual-hosted git repository.
ljain pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-ratis.git
The following commit(s) were added to refs/heads/master by this push:
new d783bc4 RATIS-799. Add a new retry policy Exception dependent retry policy. Contributed by Bharat Viswanadham.
d783bc4 is described below
commit d783bc43eb74f31cecc4dda171fbbf876230306e
Author: Lokesh Jain <lj...@apache.org>
AuthorDate: Fri Jan 31 22:50:58 2020 +0530
RATIS-799. Add a new retry policy Exception dependent retry policy. Contributed by Bharat Viswanadham.
---
.../org/apache/ratis/client/ClientRetryEvent.java | 1 +
.../ratis/retry/ExceptionDependentRetry.java | 92 +++++++++++++
.../java/org/apache/ratis/retry/RetryPolicy.java | 4 +
.../ratis/retry/TestExceptionDependentRetry.java | 151 +++++++++++++++++++++
.../org/apache/ratis/retry/TestRetryPolicy.java | 136 +++++++++++++++++++
5 files changed, 384 insertions(+)
diff --git a/ratis-client/src/main/java/org/apache/ratis/client/ClientRetryEvent.java b/ratis-client/src/main/java/org/apache/ratis/client/ClientRetryEvent.java
index 90be9fb..7056c6f 100644
--- a/ratis-client/src/main/java/org/apache/ratis/client/ClientRetryEvent.java
+++ b/ratis-client/src/main/java/org/apache/ratis/client/ClientRetryEvent.java
@@ -45,6 +45,7 @@ public class ClientRetryEvent implements RetryPolicy.Event {
return request;
}
+ @Override
public Throwable getCause() {
return cause;
}
diff --git a/ratis-common/src/main/java/org/apache/ratis/retry/ExceptionDependentRetry.java b/ratis-common/src/main/java/org/apache/ratis/retry/ExceptionDependentRetry.java
new file mode 100644
index 0000000..7465b42
--- /dev/null
+++ b/ratis-common/src/main/java/org/apache/ratis/retry/ExceptionDependentRetry.java
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ratis.retry;
+
+import org.apache.ratis.util.Preconditions;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeMap;
+
+/**
+ * Exception dependent retry policy.
+ *
+ * If exception is defined in policyMap, will use the retry policy
+ * configured to that exception or else will use the default policy.
+ */
+public final class ExceptionDependentRetry implements RetryPolicy {
+
+ public static class Builder {
+ private RetryPolicy defaultPolicy;
+ private final Map<String, RetryPolicy> exceptionNameToPolicyMap =
+ new TreeMap<>();
+
+ public Builder setExceptionToPolicy(Class<? extends Throwable> exception,
+ RetryPolicy retryPolicy) {
+ Preconditions.assertTrue(retryPolicy != null, "Exception to policy should not be null");
+ final RetryPolicy previous = exceptionNameToPolicyMap.put(exception.getName(), retryPolicy);
+ Preconditions.assertNull(previous, () -> "The exception " + exception + " is already set to " + previous);
+ return this;
+ }
+
+ public Builder setDefaultPolicy(RetryPolicy retryPolicy) {
+ Preconditions.assertTrue(retryPolicy != null, "Default Policy should not be null");
+ this.defaultPolicy = retryPolicy;
+ return this;
+ }
+
+ public ExceptionDependentRetry build() {
+ return new ExceptionDependentRetry(defaultPolicy,
+ exceptionNameToPolicyMap);
+ }
+ }
+
+ public static Builder newBuilder() {
+ return new Builder();
+ }
+
+ private final RetryPolicy defaultPolicy;
+ private final Map<String, RetryPolicy> exceptionNameToPolicyMap;
+
+
+ private ExceptionDependentRetry(RetryPolicy defaultPolicy, Map<String, RetryPolicy> policyMap) {
+ Preconditions.assertTrue(defaultPolicy != null, "Default Policy should not be null");
+ this.defaultPolicy = defaultPolicy;
+ this.exceptionNameToPolicyMap = Collections.unmodifiableMap(policyMap);
+ }
+
+ @Override
+ public Action handleAttemptFailure(Event event) {
+ RetryPolicy policy = null;
+
+ // If exception is defined in policy map use that or else go with the
+ // default one. We go with default one in 2 cases.
+ // 1. If policy map does not have exception mapped to policy.
+ // 2. If event has exception value null.
+ if (event.getCause() != null) {
+ policy = exceptionNameToPolicyMap.get(event.getCause().getClass().getName());
+ }
+
+ if (policy == null) {
+ policy = defaultPolicy;
+ }
+
+ return policy.handleAttemptFailure(event);
+ }
+}
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 743000b..fd38e4c 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
@@ -54,6 +54,10 @@ public interface RetryPolicy {
interface Event {
/** @return the number of attempts tried so far. */
int getAttemptCount();
+
+ default Throwable getCause() {
+ return null;
+ }
}
/**
diff --git a/ratis-test/src/test/java/org/apache/ratis/retry/TestExceptionDependentRetry.java b/ratis-test/src/test/java/org/apache/ratis/retry/TestExceptionDependentRetry.java
new file mode 100644
index 0000000..769245b
--- /dev/null
+++ b/ratis-test/src/test/java/org/apache/ratis/retry/TestExceptionDependentRetry.java
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ratis.retry;
+
+import org.apache.ratis.client.ClientRetryEvent;
+import org.apache.ratis.protocol.TimeoutIOException;
+import org.apache.ratis.util.TimeDuration;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Class to test {@link ExceptionDependentRetry}.
+ */
+public class TestExceptionDependentRetry {
+
+ @Test
+ public void testExceptionDependentRetrySuccess() {
+ ExceptionDependentRetry.Builder builder =
+ ExceptionDependentRetry.newBuilder();
+
+ int ioExceptionRetries = 1;
+ int timeoutExceptionRetries = 2;
+ int defaultExceptionRetries = 5;
+
+ long ioExceptionSleepTime = 1;
+ long timeoutExceptionSleepTime = 4;
+ long defaultExceptionSleepTime = 10;
+ builder.setDefaultPolicy(RetryPolicies.retryUpToMaximumCountWithFixedSleep(defaultExceptionRetries,
+ TimeDuration.valueOf(defaultExceptionSleepTime, TimeUnit.SECONDS)));
+ builder.setExceptionToPolicy(IOException.class,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(ioExceptionRetries,
+ TimeDuration.valueOf(ioExceptionSleepTime, TimeUnit.SECONDS)));
+ builder.setExceptionToPolicy(TimeoutIOException.class,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(timeoutExceptionRetries,
+ TimeDuration.valueOf(timeoutExceptionSleepTime, TimeUnit.SECONDS)));
+
+
+ ExceptionDependentRetry exceptionDependentRetry = builder.build();
+
+ testException(ioExceptionRetries, ioExceptionSleepTime,
+ exceptionDependentRetry, new IOException());
+ testException(timeoutExceptionRetries, timeoutExceptionSleepTime,
+ exceptionDependentRetry, new TimeoutIOException("time out"));
+
+ // now try with an exception which is not there in the map.
+ testException(defaultExceptionRetries, defaultExceptionSleepTime,
+ exceptionDependentRetry, new TimeoutException());
+
+ }
+
+ @Test
+ public void testExceptionDependentRetryFailureWithExceptionDuplicate() {
+
+ try {
+ ExceptionDependentRetry.Builder builder =
+ ExceptionDependentRetry.newBuilder();
+ builder.setExceptionToPolicy(IOException.class,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(1,
+ TimeDuration.valueOf(1, TimeUnit.SECONDS)));
+ builder.setExceptionToPolicy(IOException.class,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(1,
+ TimeDuration.valueOf(1, TimeUnit.SECONDS)));
+ fail("testExceptionDependentRetryFailure failed");
+ } catch (Exception ex) {
+ Assert.assertEquals(IllegalStateException.class, ex.getClass());
+ }
+
+ }
+
+ @Test
+ public void testExceptionDependentRetryFailureWithExceptionMappedToNull() {
+ try {
+ ExceptionDependentRetry.Builder builder =
+ ExceptionDependentRetry.newBuilder();
+ builder.setExceptionToPolicy(IOException.class,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(1,
+ TimeDuration.valueOf(1, TimeUnit.SECONDS)));
+ builder.setExceptionToPolicy(IOException.class, null);
+ fail("testExceptionDependentRetryFailure failed");
+ } catch (Exception ex) {
+ Assert.assertEquals(IllegalStateException.class, ex.getClass());
+ }
+ }
+
+ @Test
+ public void testExceptionDependentRetryFailureWithNoDefault() {
+
+ try {
+ ExceptionDependentRetry.Builder builder =
+ ExceptionDependentRetry.newBuilder();
+ builder.setExceptionToPolicy(IOException.class,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(1,
+ TimeDuration.valueOf(1, TimeUnit.SECONDS)));
+ builder.build();
+ fail("testExceptionDependentRetryFailureWithNoDefault failed");
+ } catch (Exception ex) {
+ Assert.assertEquals(IllegalStateException.class, ex.getClass());
+ }
+
+ try {
+ ExceptionDependentRetry.Builder builder =
+ ExceptionDependentRetry.newBuilder();
+ builder.setExceptionToPolicy(IOException.class,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(1,
+ TimeDuration.valueOf(1, TimeUnit.SECONDS)));
+ builder.setDefaultPolicy(null);
+ fail("testExceptionDependentRetryFailureWithNoDefault failed");
+ } catch (Exception ex) {
+ Assert.assertEquals(IllegalStateException.class, ex.getClass());
+ }
+ }
+
+ private void testException(int retries, long sleepTime,
+ ExceptionDependentRetry exceptionDependentRetry, Exception exception) {
+ for (int i = 0; i < retries + 1; i++) {
+ RetryPolicy.Action action =
+ exceptionDependentRetry.handleAttemptFailure(new ClientRetryEvent(i,
+ null, exception));
+
+ final boolean expected = i < retries;
+ Assert.assertEquals(expected, action.shouldRetry());
+ if (expected) {
+ Assert.assertEquals(sleepTime, action.getSleepTime().getDuration());
+ } else {
+ Assert.assertEquals(0L, action.getSleepTime().getDuration());
+ }
+ }
+ }
+}
diff --git a/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java b/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java
index 7cfc403..31bedd9 100644
--- a/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java
+++ b/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java
@@ -23,13 +23,25 @@ import org.apache.ratis.client.retry.RequestTypeDependentRetryPolicy;
import org.apache.ratis.proto.RaftProtos.RaftClientRequestProto;
import org.apache.ratis.proto.RaftProtos.ReplicationLevel;
import org.apache.ratis.protocol.ClientId;
+import org.apache.ratis.protocol.LeaderNotReadyException;
+import org.apache.ratis.protocol.NotLeaderException;
import org.apache.ratis.protocol.RaftClientRequest;
import org.apache.ratis.protocol.RaftGroupId;
+import org.apache.ratis.protocol.RaftGroupMemberId;
import org.apache.ratis.protocol.RaftPeerId;
+import org.apache.ratis.protocol.TimeoutIOException;
+import org.apache.ratis.protocol.exceptions.ResourceUnavailableException;
import org.apache.ratis.util.TimeDuration;
import org.junit.Assert;
import org.junit.Test;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
/** Test {@link RetryPolicy}. */
public class TestRetryPolicy extends BaseTest {
@Override
@@ -111,7 +123,131 @@ public class TestRetryPolicy extends BaseTest {
}
+ @Test
+ public void testRequestTypeDependentRetryWithExceptionDependentPolicy() throws Exception {
+ final RequestTypeDependentRetryPolicy.Builder retryPolicy =
+ RequestTypeDependentRetryPolicy.newBuilder();
+ Map<Class<? extends Throwable>, Pair> exceptionPolicyMap = new HashMap<>();
+ exceptionPolicyMap.put(NotLeaderException.class, new Pair(10, 1));
+ exceptionPolicyMap.put(LeaderNotReadyException.class, new Pair(10, 1));
+ exceptionPolicyMap.put(TimeoutIOException.class, new Pair(5, 5));
+ exceptionPolicyMap.put(ResourceUnavailableException.class, new Pair(5, 5));
+ Pair defaultPolicy = new Pair(10, 2);
+
+ retryPolicy.set(RaftClientRequestProto.TypeCase.WRITE, buildExceptionBasedRetry(exceptionPolicyMap, defaultPolicy));
+ retryPolicy.set(RaftClientRequestProto.TypeCase.WATCH, buildExceptionBasedRetry(exceptionPolicyMap, defaultPolicy));
+
+ final RetryPolicy policy = retryPolicy.build();
+ LOG.info("policy = {}", policy);
+
+ final RaftClientRequest writeRequest = newRaftClientRequest(RaftClientRequest.writeRequestType());
+ final RaftClientRequest watchRequest = newRaftClientRequest(
+ RaftClientRequest.watchRequestType(1, ReplicationLevel.MAJORITY));
+
+ List<RaftClientRequest> requests = new ArrayList<>();
+ requests.add(writeRequest);
+ requests.add(watchRequest);
+
+ for (RaftClientRequest raftClientRequest : requests) {
+ for (Map.Entry< Class< ? extends Throwable >, Pair > exceptionPolicy :
+ exceptionPolicyMap.entrySet()) {
+ Throwable exception = createException(exceptionPolicy.getKey());
+ for (int j = 1; j < exceptionPolicy.getValue().retries * 2; j++) {
+ checkEvent(j, policy, raftClientRequest, exception,
+ exceptionPolicy.getValue());
+ }
+ }
+ }
+ // Now try with an exception that is not defined in retry exception map.
+ for (RaftClientRequest raftClientRequest : requests) {
+ Throwable exception = createException(IOException.class);
+ for (int j = 1; j < defaultPolicy.retries * 2; j++) {
+ checkEvent(j, policy, raftClientRequest, exception, defaultPolicy);
+ }
+ }
+ }
+
+ /**
+ * Check Event with the policy defined.
+ * @param exceptionAttemptCount
+ * @param retryPolicy
+ * @param raftClientRequest
+ * @param exception
+ * @param exceptionPolicyPair
+ */
+ private void checkEvent(int exceptionAttemptCount, RetryPolicy retryPolicy, RaftClientRequest raftClientRequest,
+ Throwable exception, Pair exceptionPolicyPair) {
+ final ClientRetryEvent event = new ClientRetryEvent(exceptionAttemptCount, raftClientRequest, exception);
+ final RetryPolicy.Action action = retryPolicy.handleAttemptFailure(event);
+
+ final boolean expected = exceptionAttemptCount < exceptionPolicyPair.retries;
+ Assert.assertEquals(expected, action.shouldRetry());
+ if (expected) {
+ Assert.assertEquals(exceptionPolicyPair.sleepTime, action.getSleepTime().getDuration());
+ } else {
+ Assert.assertEquals(0L, action.getSleepTime().getDuration());
+ }
+ }
+
+ /**
+ * Create exception object based on the exception class,
+ * @param exception
+ * @return exception object.
+ */
+ private Throwable createException(Class<? extends Throwable> exception) {
+ Throwable ex;
+ if (exception.getName().equals(LeaderNotReadyException.class.getName())) {
+ ex =
+ new LeaderNotReadyException(RaftGroupMemberId.valueOf(RaftPeerId.valueOf("node1"), RaftGroupId.randomId()));
+ } else if (exception.getName().equals(NotLeaderException.class.getName())) {
+ ex = new NotLeaderException(null, null, null);
+ } else if (exception.getName().equals(TimeoutIOException.class.getName())) {
+ ex = new TimeoutIOException("time out");
+ } else if (exception.getName().equals(ResourceUnavailableException.class.getName())){
+ ex = new ResourceUnavailableException("resource unavailable");
+ } else {
+ ex = new IOException("io exception");
+ }
+ return ex;
+ }
+
+ /**
+ * Build {@link ExceptionDependentRetry} object.
+ * @param exceptionPolicyMap
+ * @param defaultPolicy
+ * @return ExceptionDependentRetry
+ */
+ private ExceptionDependentRetry buildExceptionBasedRetry(Map<Class<?
+ extends Throwable>, Pair> exceptionPolicyMap, Pair defaultPolicy) {
+ final ExceptionDependentRetry.Builder policy =
+ ExceptionDependentRetry.newBuilder();
+ exceptionPolicyMap.forEach((k, v) -> {
+ policy.setExceptionToPolicy(k,
+ RetryPolicies.retryUpToMaximumCountWithFixedSleep(v.retries,
+ TimeDuration.valueOf(v.sleepTime, TimeUnit.SECONDS)));
+ });
+
+ policy.setDefaultPolicy(RetryPolicies.retryUpToMaximumCountWithFixedSleep(defaultPolicy.retries,
+ TimeDuration.valueOf(defaultPolicy.sleepTime, TimeUnit.SECONDS)));
+
+ return policy.build();
+ }
+
+
private static RaftClientRequest newRaftClientRequest(RaftClientRequest.Type type) {
return new RaftClientRequest(ClientId.randomId(), RaftPeerId.valueOf("s0"), RaftGroupId.randomId(), 1L, type);
}
+
+ /**
+ * Pair class to hold retries and sleep time.
+ */
+ static class Pair {
+ private int retries;
+ private long sleepTime;
+
+ Pair(int retries, long sleepTime) {
+ this.retries = retries;
+ this.sleepTime = sleepTime;
+ }
+ }
}
\ No newline at end of file