This is an automated email from the ASF dual-hosted git repository.
albumenj pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/master by this push:
new e6148ac fix: Fix the problem of wrong number of retries in failback
mode (#9525)
e6148ac is described below
commit e6148ac06e2cca6b2705fd590d5dead1e15e1cd7
Author: 桔子 <[email protected]>
AuthorDate: Sun Jan 23 16:58:22 2022 +0800
fix: Fix the problem of wrong number of retries in failback mode (#9525)
When the retries value is zero,
the logic in failback mode resets it to the default value of 3,
making it impossible to turn off the retry mechanism.
Fixes #9522
---
.../cluster/support/FailbackClusterInvoker.java | 21 ++++-
.../support/FailbackClusterInvokerTest.java | 96 ++++++++++++++++++++++
2 files changed, 113 insertions(+), 4 deletions(-)
diff --git
a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java
b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java
index 0d2d6d1..cbbe853 100644
---
a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java
+++
b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java
@@ -52,6 +52,9 @@ public class FailbackClusterInvoker<T> extends
AbstractClusterInvoker<T> {
private static final long RETRY_FAILED_PERIOD = 5;
+ /**
+ * Number of retries obtained from the configuration, don't contain the
first invoke.
+ */
private final int retries;
private final int failbackTasks;
@@ -62,7 +65,7 @@ public class FailbackClusterInvoker<T> extends
AbstractClusterInvoker<T> {
super(directory);
int retriesConfig = getUrl().getParameter(RETRIES_KEY,
DEFAULT_FAILBACK_TIMES);
- if (retriesConfig <= 0) {
+ if (retriesConfig < 0) {
retriesConfig = DEFAULT_FAILBACK_TIMES;
}
int failbackTasksConfig = getUrl().getParameter(FAIL_BACK_TASKS_KEY,
DEFAULT_FAILBACK_TASKS);
@@ -124,10 +127,18 @@ public class FailbackClusterInvoker<T> extends
AbstractClusterInvoker<T> {
private final Invocation invocation;
private final LoadBalance loadbalance;
private final List<Invoker<T>> invokers;
- private final int retries;
private final long tick;
private Invoker<T> lastInvoker;
- private int retryTimes = 0;
+
+ /**
+ * Number of retries obtained from the configuration, don't contain
the first invoke.
+ */
+ private final int retries;
+
+ /**
+ * Number of retried.
+ */
+ private int retriedTimes = 0;
RetryTimerTask(LoadBalance loadbalance, Invocation invocation,
List<Invoker<T>> invokers, Invoker<T> lastInvoker, int retries, long tick) {
this.loadbalance = loadbalance;
@@ -141,12 +152,14 @@ public class FailbackClusterInvoker<T> extends
AbstractClusterInvoker<T> {
@Override
public void run(Timeout timeout) {
try {
+ logger.info("Attempt to retry to invoke method " +
invocation.getMethodName() +
+ ". The total will retry " + retries + " times, the
current is the " + retriedTimes + " retry");
Invoker<T> retryInvoker = select(loadbalance, invocation,
invokers, Collections.singletonList(lastInvoker));
lastInvoker = retryInvoker;
retryInvoker.invoke(invocation);
} catch (Throwable e) {
logger.error("Failed retry to invoke method " +
invocation.getMethodName() + ", waiting again.", e);
- if ((++retryTimes) >= retries) {
+ if ((++retriedTimes) >= retries) {
logger.error("Failed retry times exceed threshold (" +
retries + "), We have to abandon, invocation->" + invocation);
} else {
rePut(timeout);
diff --git
a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvokerTest.java
b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvokerTest.java
index d648943..489a921 100644
---
a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvokerTest.java
+++
b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvokerTest.java
@@ -18,6 +18,7 @@
package org.apache.dubbo.rpc.cluster.support;
import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.constants.CommonConstants;
import org.apache.dubbo.common.utils.DubboAppender;
import org.apache.dubbo.common.utils.LogUtil;
import org.apache.dubbo.rpc.AppResponse;
@@ -37,11 +38,13 @@ import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
+import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import static org.apache.dubbo.common.constants.CommonConstants.RETRIES_KEY;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
@@ -174,4 +177,97 @@ public class FailbackClusterInvokerTest {
Assertions.assertEquals(1, LogUtil.findMessage(Level.ERROR, "Failback
background works error"), "must have one error message ");
// it can be invoke successfully
}
+
+
+ private long getRetryFailedPeriod() throws NoSuchFieldException,
IllegalAccessException {
+ Field retryFailedPeriod =
FailbackClusterInvoker.class.getDeclaredField("RETRY_FAILED_PERIOD");
+ retryFailedPeriod.setAccessible(true);
+ return retryFailedPeriod.getLong(FailbackClusterInvoker.class);
+ }
+
+ @Test
+ @Order(5)
+ public void testInvokeRetryTimesWithZeroValue() throws
InterruptedException, NoSuchFieldException,
+ IllegalAccessException {
+ int retries = 0;
+ resetInvokerToException();
+ given(dic.getConsumerUrl()).willReturn(url.addParameter(RETRIES_KEY,
retries));
+
+ FailbackClusterInvoker<FailbackClusterInvokerTest> invoker = new
FailbackClusterInvoker<>(dic);
+ LogUtil.start();
+ DubboAppender.clear();
+
+ invocation.setMethodName("testInvokeRetryTimesWithZeroValue");
+ invoker.invoke(invocation);
+
+ CountDownLatch countDown = new CountDownLatch(1);
+ countDown.await(getRetryFailedPeriod() * (retries + 1),
TimeUnit.SECONDS);
+ LogUtil.stop();
+ Assertions.assertEquals(0, LogUtil.findMessage(Level.INFO, "Attempt to
retry to invoke method " +
+ "testInvokeRetryTimesWithZeroValue"), "No retry messages
allowed");
+ }
+
+ @Test
+ @Order(6)
+ public void testInvokeRetryTimesWithTwoValue() throws
InterruptedException, NoSuchFieldException,
+ IllegalAccessException {
+ int retries = 2;
+ resetInvokerToException();
+ given(dic.getConsumerUrl()).willReturn(url.addParameter(RETRIES_KEY,
retries));
+
+ FailbackClusterInvoker<FailbackClusterInvokerTest> invoker = new
FailbackClusterInvoker<>(dic);
+ LogUtil.start();
+ DubboAppender.clear();
+
+ invocation.setMethodName("testInvokeRetryTimesWithTwoValue");
+ invoker.invoke(invocation);
+
+ CountDownLatch countDown = new CountDownLatch(1);
+ countDown.await(getRetryFailedPeriod() * (retries + 1),
TimeUnit.SECONDS);
+ LogUtil.stop();
+ Assertions.assertEquals(2, LogUtil.findMessage(Level.INFO, "Attempt to
retry to invoke method " +
+ "testInvokeRetryTimesWithTwoValue"), "Must have two error
message ");
+ }
+
+ @Test
+ @Order(7)
+ public void testInvokeRetryTimesWithDefaultValue() throws
InterruptedException, NoSuchFieldException,
+ IllegalAccessException {
+ resetInvokerToException();
+
given(dic.getConsumerUrl()).willReturn(URL.valueOf("test://test:11/test"));
+
+ FailbackClusterInvoker<FailbackClusterInvokerTest> invoker = new
FailbackClusterInvoker<>(dic);
+ LogUtil.start();
+ DubboAppender.clear();
+
+ invocation.setMethodName("testInvokeRetryTimesWithDefaultValue");
+ invoker.invoke(invocation);
+
+ CountDownLatch countDown = new CountDownLatch(1);
+ countDown.await(getRetryFailedPeriod() *
(CommonConstants.DEFAULT_FAILBACK_TIMES + 1), TimeUnit.SECONDS);
+ LogUtil.stop();
+ Assertions.assertEquals(3, LogUtil.findMessage(Level.INFO, "Attempt to
retry to invoke method " +
+ "testInvokeRetryTimesWithDefaultValue"), "Must have three
error message ");
+ }
+
+ @Test
+ @Order(8)
+ public void testInvokeRetryTimesWithIllegalValue() throws
InterruptedException, NoSuchFieldException,
+ IllegalAccessException {
+ resetInvokerToException();
+ given(dic.getConsumerUrl()).willReturn(url.addParameter(RETRIES_KEY,
-100));
+
+ FailbackClusterInvoker<FailbackClusterInvokerTest> invoker = new
FailbackClusterInvoker<>(dic);
+ LogUtil.start();
+ DubboAppender.clear();
+
+ invocation.setMethodName("testInvokeRetryTimesWithIllegalValue");
+ invoker.invoke(invocation);
+
+ CountDownLatch countDown = new CountDownLatch(1);
+ countDown.await(getRetryFailedPeriod() *
(CommonConstants.DEFAULT_FAILBACK_TIMES + 1), TimeUnit.SECONDS);
+ LogUtil.stop();
+ Assertions.assertEquals(3, LogUtil.findMessage(Level.INFO, "Attempt to
retry to invoke method " +
+ "testInvokeRetryTimesWithIllegalValue"), "Must have three
error message ");
+ }
}
\ No newline at end of file
