From 573c0ca36be3a816dbd5bb81711d09152939f91f Mon Sep 17 00:00:00 2001 From: zhuyong Date: Mon, 7 May 2018 15:35:49 +0800 Subject: [PATCH 1/2] fix #934 use loadBalance policy to choose invoke when providers less than 2 --- .../dubbo/rpc/cluster/support/AbstractClusterInvoker.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java b/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java index 68ed406f9cd..c14d5fc210b 100644 --- a/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java +++ b/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java @@ -136,10 +136,6 @@ private Invoker doSelect(LoadBalance loadbalance, Invocation invocation, List return null; if (invokers.size() == 1) return invokers.get(0); - // If we only have two invokers, use round-robin instead. - if (invokers.size() == 2 && selected != null && !selected.isEmpty()) { - return selected.get(0) == invokers.get(0) ? invokers.get(1) : invokers.get(0); - } if (loadbalance == null) { loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(Constants.DEFAULT_LOADBALANCE); } From b470023c41cb1f15ec7d66168869cd6a1ecece14 Mon Sep 17 00:00:00 2001 From: zhuyong Date: Wed, 9 May 2018 13:08:10 +0800 Subject: [PATCH 2/2] fix #1756, clear mock invocation after invoking --- .../support/AbstractClusterInvoker.java | 2 +- .../support/AbstractClusterInvokerTest.java | 24 +++++++++++++++---- .../support/FailoverClusterInvokerTest.java | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java b/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java index c14d5fc210b..0a3e0a51205 100644 --- a/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java +++ b/dubbo-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java @@ -153,7 +153,7 @@ private Invoker doSelect(LoadBalance loadbalance, Invocation invocation, List int index = invokers.indexOf(invoker); try { //Avoid collision - invoker = index < invokers.size() - 1 ? invokers.get(index + 1) : invoker; + invoker = index < invokers.size() - 1 ? invokers.get(index + 1) : invokers.get(0); } catch (Exception e) { logger.warn(e.getMessage() + " may because invokers list dynamic change, ignore.", e); } diff --git a/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvokerTest.java b/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvokerTest.java index cdfcdcf8836..611683ce000 100644 --- a/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvokerTest.java +++ b/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvokerTest.java @@ -37,6 +37,7 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.Mockito; import java.util.ArrayList; import java.util.List; @@ -156,19 +157,19 @@ public void testSelect_Invokersize1() throws Exception { @Test public void testSelect_Invokersize2AndselectNotNull() throws Exception { invokers.clear(); - invokers.add(invoker1); invokers.add(invoker2); + invokers.add(invoker4); { selectedInvokers.clear(); - selectedInvokers.add(invoker1); - Invoker invoker = cluster.select(null, null, invokers, selectedInvokers); + selectedInvokers.add(invoker4); + Invoker invoker = cluster.select(null, invocation, invokers, selectedInvokers); Assert.assertEquals(invoker2, invoker); } { selectedInvokers.clear(); selectedInvokers.add(invoker2); - Invoker invoker = cluster.select(null, null, invokers, selectedInvokers); - Assert.assertEquals(invoker1, invoker); + Invoker invoker = cluster.select(null, invocation, invokers, selectedInvokers); + Assert.assertEquals(invoker4, invoker); } } @@ -322,18 +323,24 @@ public void testSelect_multiInvokers(String lbname) throws Exception { for (int i = 0; i < runs; i++) { Invoker sinvoker = cluster.select(lb, invocation, invokers, selectedInvokers); Assert.assertEquals(true, sinvoker.isAvailable()); + + Mockito.clearInvocations(invoker1, invoker2, invoker3, invoker4, invoker5); } for (int i = 0; i < runs; i++) { selectedInvokers.clear(); selectedInvokers.add(invoker1); Invoker sinvoker = cluster.select(lb, invocation, invokers, selectedInvokers); Assert.assertEquals(true, sinvoker.isAvailable()); + + Mockito.clearInvocations(invoker1, invoker2, invoker3, invoker4, invoker5); } for (int i = 0; i < runs; i++) { selectedInvokers.clear(); selectedInvokers.add(invoker2); Invoker sinvoker = cluster.select(lb, invocation, invokers, selectedInvokers); Assert.assertEquals(true, sinvoker.isAvailable()); + + Mockito.clearInvocations(invoker1, invoker2, invoker3, invoker4, invoker5); } for (int i = 0; i < runs; i++) { selectedInvokers.clear(); @@ -341,6 +348,8 @@ public void testSelect_multiInvokers(String lbname) throws Exception { selectedInvokers.add(invoker4); Invoker sinvoker = cluster.select(lb, invocation, invokers, selectedInvokers); Assert.assertEquals(true, sinvoker.isAvailable()); + + Mockito.clearInvocations(invoker1, invoker2, invoker3, invoker4, invoker5); } for (int i = 0; i < runs; i++) { selectedInvokers.clear(); @@ -349,14 +358,19 @@ public void testSelect_multiInvokers(String lbname) throws Exception { selectedInvokers.add(invoker5); Invoker sinvoker = cluster.select(lb, invocation, invokers, selectedInvokers); Assert.assertEquals(true, sinvoker.isAvailable()); + + Mockito.clearInvocations(invoker1, invoker2, invoker3, invoker4, invoker5); } for (int i = 0; i < runs; i++) { + selectedInvokers.clear(); selectedInvokers.add(invoker1); selectedInvokers.add(invoker2); selectedInvokers.add(invoker3); Invoker sinvoker = cluster.select(lb, invocation, invokers, selectedInvokers); Assert.assertEquals(true, sinvoker.isAvailable()); + + Mockito.clearInvocations(invoker1, invoker2, invoker3, invoker4, invoker5); } } diff --git a/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/FailoverClusterInvokerTest.java b/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/FailoverClusterInvokerTest.java index f89ef6fc6aa..f41864ffb39 100644 --- a/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/FailoverClusterInvokerTest.java +++ b/dubbo-cluster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/FailoverClusterInvokerTest.java @@ -135,7 +135,7 @@ public void testInvoke_retryTimes() { assertSame(result, ret); fail(); } catch (RpcException expected) { - assertTrue(expected.isTimeout()); + assertTrue((expected.isTimeout() || expected.getCode() == 0)); assertTrue(expected.getMessage().indexOf((retries + 1) + " times") > 0); } }