Fix apache#934 apache#1756 use loadBalance policy to choose invoke wh…

…en providers less 2 (apache#1759) * fix apache#934 use loadBalance policy to choose invoke when providers less than 2 * fix apache#1756, clear mock invocation after invoking
rolandhe · May 9, 2018 · e5102b9 · e5102b9
1 parent be072bf
commit e5102b9
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 11 deletions.
diff --git a/...o-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java b/...o-cluster/src/main/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvoker.java
@@ -136,10 +136,6 @@ private Invoker<T> 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);
         }
@@ -157,7 +153,7 @@ private Invoker<T> 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/...uster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/AbstractClusterInvokerTest.java b/...uster/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,25 +323,33 @@ 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();
             selectedInvokers.add(invoker2);
             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/...uster/src/test/java/com/alibaba/dubbo/rpc/cluster/support/FailoverClusterInvokerTest.java b/...uster/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);
         }
     }