From 1138b9749259398506c6cda88cfe5ceef32e48b7 Mon Sep 17 00:00:00 2001 From: PiteXChen <44110731+RapperCL@users.noreply.github.com> Date: Tue, 25 Apr 2023 11:49:16 +0800 Subject: [PATCH] fix: Adjusting cluster invoker checks (#12139) * Adjustment check(#12138) * Unit test optimization * Code optimization --- .../support/AbstractClusterInvoker.java | 2 ++ .../support/BroadcastClusterInvoker.java | 1 - .../support/FailbackClusterInvoker.java | 1 - .../support/FailfastClusterInvoker.java | 1 - .../support/FailoverClusterInvoker.java | 1 - .../support/FailsafeClusterInvoker.java | 1 - .../support/ForkingClusterInvoker.java | 1 - .../support/MergeableClusterInvoker.java | 1 - .../support/AvailableClusterInvokerTest.java | 2 +- .../support/FailSafeClusterInvokerTest.java | 15 ++++++----- .../support/FailbackClusterInvokerTest.java | 26 ++++++++++++------- .../registry/ZoneAwareClusterInvokerTest.java | 2 ++ 12 files changed, 30 insertions(+), 24 deletions(-) diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java index b03447676d..43c9e9b294 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java @@ -333,6 +333,8 @@ public abstract class AbstractClusterInvoker implements ClusterInvoker { List> invokers = list(invocation); InvocationProfilerUtils.releaseDetailProfiler(invocation); + checkInvokers(invokers, invocation); + LoadBalance loadbalance = initLoadBalance(invokers, invocation); RpcUtils.attachInvocationIdIfAsync(getUrl(), invocation); diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java index 02b8f6d2d0..bdf8f6f02e 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java @@ -51,7 +51,6 @@ public class BroadcastClusterInvoker extends AbstractClusterInvoker { @Override @SuppressWarnings({"unchecked", "rawtypes"}) public Result doInvoke(final Invocation invocation, List> invokers, LoadBalance loadbalance) throws RpcException { - checkInvokers(invokers, invocation); RpcContext.getServiceContext().setInvokers((List) invokers); RpcException exception = null; Result result = null; 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 608b00ef1f..368244a1df 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 @@ -104,7 +104,6 @@ public class FailbackClusterInvoker extends AbstractClusterInvoker { Invoker invoker = null; URL consumerUrl = RpcContext.getServiceContext().getConsumerUrl(); try { - checkInvokers(invokers, invocation); invoker = select(loadbalance, invocation, invokers, null); // Asynchronous call method must be used here, because failback will retry in the background. // Then the serviceContext will be cleared after the call is completed. diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailfastClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailfastClusterInvoker.java index 0b9f6bd2dc..22c708f824 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailfastClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailfastClusterInvoker.java @@ -41,7 +41,6 @@ public class FailfastClusterInvoker extends AbstractClusterInvoker { @Override public Result doInvoke(Invocation invocation, List> invokers, LoadBalance loadbalance) throws RpcException { - checkInvokers(invokers, invocation); Invoker invoker = select(loadbalance, invocation, invokers, null); try { return invokeWithContext(invoker, invocation); diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailoverClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailoverClusterInvoker.java index 18583742ba..b0cdb8ff53 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailoverClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailoverClusterInvoker.java @@ -57,7 +57,6 @@ public class FailoverClusterInvoker extends AbstractClusterInvoker { @SuppressWarnings({"unchecked", "rawtypes"}) public Result doInvoke(Invocation invocation, final List> invokers, LoadBalance loadbalance) throws RpcException { List> copyInvokers = invokers; - checkInvokers(copyInvokers, invocation); String methodName = RpcUtils.getMethodName(invocation); int len = calculateInvokeTimes(methodName); // retry loop. diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailsafeClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailsafeClusterInvoker.java index c09205bdbd..78c3540895 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailsafeClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailsafeClusterInvoker.java @@ -47,7 +47,6 @@ public class FailsafeClusterInvoker extends AbstractClusterInvoker { @Override public Result doInvoke(Invocation invocation, List> invokers, LoadBalance loadbalance) throws RpcException { try { - checkInvokers(invokers, invocation); Invoker invoker = select(loadbalance, invocation, invokers, null); return invokeWithContext(invoker, invocation); } catch (Throwable e) { diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/ForkingClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/ForkingClusterInvoker.java index a1334aab07..b2f0d2e095 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/ForkingClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/ForkingClusterInvoker.java @@ -67,7 +67,6 @@ public class ForkingClusterInvoker extends AbstractClusterInvoker { @SuppressWarnings({"unchecked", "rawtypes"}) public Result doInvoke(final Invocation invocation, List> invokers, LoadBalance loadbalance) throws RpcException { try { - checkInvokers(invokers, invocation); final List> selected; final int forks = getUrl().getParameter(FORKS_KEY, DEFAULT_FORKS); final int timeout = getUrl().getParameter(TIMEOUT_KEY, DEFAULT_TIMEOUT); diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java index bdd2ddd6d2..6d61eb2194 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/MergeableClusterInvoker.java @@ -59,7 +59,6 @@ public class MergeableClusterInvoker extends AbstractClusterInvoker { @Override protected Result doInvoke(Invocation invocation, List> invokers, LoadBalance loadbalance) throws RpcException { - checkInvokers(invokers, invocation); String merger = getUrl().getMethodParameter(invocation.getMethodName(), MERGER_KEY); if (ConfigUtils.isEmpty(merger)) { // If a method doesn't have a merger, only invoke one Group for (final Invoker invoker : invokers) { diff --git a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/AvailableClusterInvokerTest.java b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/AvailableClusterInvokerTest.java index 4218bd72d8..b3357e9ba4 100644 --- a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/AvailableClusterInvokerTest.java +++ b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/AvailableClusterInvokerTest.java @@ -107,7 +107,7 @@ class AvailableClusterInvokerTest { invoker.invoke(invocation); fail(); } catch (RpcException e) { - Assertions.assertTrue(e.getMessage().contains("No provider available in")); + Assertions.assertTrue(e.getMessage().contains("No provider available")); assertFalse(e.getCause() instanceof RpcException); } } diff --git a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailSafeClusterInvokerTest.java b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailSafeClusterInvokerTest.java index 8887154a6c..dc05557f20 100644 --- a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailSafeClusterInvokerTest.java +++ b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/FailSafeClusterInvokerTest.java @@ -17,7 +17,6 @@ package org.apache.dubbo.rpc.cluster.support; import org.apache.dubbo.common.URL; -import org.apache.dubbo.common.utils.LogUtil; import org.apache.dubbo.rpc.AppResponse; import org.apache.dubbo.rpc.Invoker; import org.apache.dubbo.rpc.Result; @@ -25,6 +24,7 @@ import org.apache.dubbo.rpc.RpcContext; import org.apache.dubbo.rpc.RpcInvocation; import org.apache.dubbo.rpc.cluster.Directory; import org.apache.dubbo.rpc.cluster.filter.DemoService; +import org.apache.dubbo.rpc.RpcException; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -33,7 +33,7 @@ import org.junit.jupiter.api.Test; import java.util.ArrayList; import java.util.List; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -113,10 +113,13 @@ class FailSafeClusterInvokerTest { resetInvokerToNoException(); FailsafeClusterInvoker invoker = new FailsafeClusterInvoker(dic); - LogUtil.start(); - invoker.invoke(invocation); - assertTrue(LogUtil.findMessage("No provider") > 0); - LogUtil.stop(); + + try{ + invoker.invoke(invocation); + } catch (RpcException e){ + Assertions.assertTrue(e.getMessage().contains("No provider available")); + assertFalse(e.getCause() instanceof RpcException); + } } } 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 d7e48afef6..ed161d7c79 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 @@ -24,9 +24,10 @@ import org.apache.dubbo.common.utils.LogUtil; import org.apache.dubbo.rpc.AppResponse; import org.apache.dubbo.rpc.Invoker; import org.apache.dubbo.rpc.Result; -import org.apache.dubbo.rpc.RpcContext; import org.apache.dubbo.rpc.RpcInvocation; +import org.apache.dubbo.rpc.RpcContext; import org.apache.dubbo.rpc.cluster.Directory; +import org.apache.dubbo.rpc.RpcException; import org.apache.log4j.Level; import org.junit.jupiter.api.AfterEach; @@ -37,6 +38,7 @@ import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.function.Executable; import java.lang.reflect.Field; import java.util.ArrayList; @@ -45,7 +47,7 @@ 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.junit.jupiter.api.Assertions.*; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -110,6 +112,9 @@ class FailbackClusterInvokerTest { given(dic.getUrl()).willReturn(url); given(dic.getConsumerUrl()).willReturn(url); given(dic.getInterface()).willReturn(FailbackClusterInvokerTest.class); + given(dic.list(invocation)).willReturn(invokers); + given(invoker.getUrl()).willReturn(url); + FailbackClusterInvoker invoker = new FailbackClusterInvoker<>(dic); invoker.invoke(invocation); Assertions.assertNull(RpcContext.getServiceContext().getInvoker()); @@ -123,6 +128,9 @@ class FailbackClusterInvokerTest { given(dic.getUrl()).willReturn(url); given(dic.getConsumerUrl()).willReturn(url); given(dic.getInterface()).willReturn(FailbackClusterInvokerTest.class); + given(dic.list(invocation)).willReturn(invokers); + given(invoker.getUrl()).willReturn(url); + FailbackClusterInvoker invoker = new FailbackClusterInvoker<>(dic); invoker.invoke(invocation); Assertions.assertNull(RpcContext.getServiceContext().getInvoker()); @@ -161,17 +169,15 @@ class FailbackClusterInvokerTest { given(dic.getInterface()).willReturn(FailbackClusterInvokerTest.class); invocation.setMethodName("method1"); - invokers.add(invoker); - resetInvokerToNoException(); - FailbackClusterInvoker invoker = new FailbackClusterInvoker<>(dic); - LogUtil.start(); - DubboAppender.clear(); - invoker.invoke(invocation); - assertEquals(1, LogUtil.findMessage("Failback to invoke")); - LogUtil.stop(); + try{ + invoker.invoke(invocation); + } catch (RpcException e){ + Assertions.assertTrue(e.getMessage().contains("No provider available")); + assertFalse(e.getCause() instanceof RpcException); + } } @Disabled diff --git a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/registry/ZoneAwareClusterInvokerTest.java b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/registry/ZoneAwareClusterInvokerTest.java index 021f6facb4..9a20078935 100644 --- a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/registry/ZoneAwareClusterInvokerTest.java +++ b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/registry/ZoneAwareClusterInvokerTest.java @@ -24,6 +24,7 @@ import org.apache.dubbo.rpc.RpcException; import org.apache.dubbo.rpc.cluster.ClusterInvoker; import org.apache.dubbo.rpc.cluster.Directory; +import org.apache.dubbo.rpc.cluster.support.AbstractClusterInvoker; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -174,6 +175,7 @@ class ZoneAwareClusterInvokerTest { given(directory.getUrl()).willReturn(url); given(directory.getConsumerUrl()).willReturn(url); given(directory.list(invocation)).willReturn(new ArrayList<>(0)); + given(directory.getInterface()).willReturn(ZoneAwareClusterInvokerTest.class); zoneAwareClusterInvoker = new ZoneAwareClusterInvoker<>(directory);