You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by al...@apache.org on 2022/02/06 03:57:35 UTC
[dubbo] branch 3.0 updated: fix: Fix the problem of wrong number of retries in failback mode (#9526)
This is an automated email from the ASF dual-hosted git repository.
albumenj pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/3.0 by this push:
new c6109cd fix: Fix the problem of wrong number of retries in failback mode (#9526)
c6109cd is described below
commit c6109cda0803525c382a8484d12cd237cb6a214d
Author: 桔子 <ju...@qq.com>
AuthorDate: Sun Feb 6 11:57:20 2022 +0800
fix: Fix the problem of wrong number of retries in failback mode (#9526)
* fix: Fix the problem of wrong number of retries in failback mode
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
* refactor: Remove FailbackClusterInvoker unused field
---
.../cluster/support/FailbackClusterInvoker.java | 21 ++++-
.../support/FailbackClusterInvokerTest.java | 97 ++++++++++++++++++++++
2 files changed, 114 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 e934383..120f594 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
@@ -54,6 +54,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;
@@ -64,7 +67,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);
@@ -129,12 +132,20 @@ 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;
private URL consumerUrl;
+ /**
+ * 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, URL consumerUrl) {
this.loadbalance = loadbalance;
@@ -149,12 +160,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;
invokeWithContextAsync(retryInvoker, invocation, consumerUrl);
} 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 64b3086..7711861 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;
@@ -196,4 +199,98 @@ 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 ");
+ }
}