You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2021/05/26 07:21:14 UTC

[servicecomb-java-chassis] branch master updated: [SCB-2266]service center client use RoundRobin rule (#2391)

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git


The following commit(s) were added to refs/heads/master by this push:
     new 07a3708  [SCB-2266]service center client use RoundRobin rule (#2391)
07a3708 is described below

commit 07a3708e7780d2cba89b559a886b2d96cbfb798e
Author: wxkwxkwxk1231 <ka...@protonmail.com>
AuthorDate: Wed May 26 15:21:05 2021 +0800

    [SCB-2266]service center client use RoundRobin rule (#2391)
---
 .../serviceregistry/client/IpPortManager.java      | 14 +-----
 .../client/http/ServiceRegistryClientImpl.java     |  6 ++-
 .../serviceregistry/client/TestIpPortManager.java  | 56 ++++++++--------------
 .../client/http/TestClientHttp.java                |  3 +-
 4 files changed, 27 insertions(+), 52 deletions(-)

diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/IpPortManager.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/IpPortManager.java
index 772f157..e9728bf 100644
--- a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/IpPortManager.java
+++ b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/IpPortManager.java
@@ -91,20 +91,8 @@ public class IpPortManager {
     }
   }
 
-  public IpPort getNextAvailableAddress(IpPort failedIpPort) {
-    int currentIndex = currentAvailableIndex.get();
-    IpPort current = getAvailableAddress(currentIndex);
-    if (current.equals(failedIpPort)) {
-      currentAvailableIndex.compareAndSet(currentIndex, currentIndex + 1);
-      current = getAvailableAddress();
-    }
-
-    LOGGER.info("Change service center address from {} to {}", failedIpPort.toString(), current.toString());
-    return current;
-  }
-
   public IpPort getAvailableAddress() {
-    return getAvailableAddress(currentAvailableIndex.get());
+    return getAvailableAddress(currentAvailableIndex.incrementAndGet());
   }
 
   private IpPort getAvailableAddress(int index) {
diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java
index bc8ed80..9b38300 100644
--- a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java
+++ b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java
@@ -120,8 +120,10 @@ public final class ServiceRegistryClientImpl implements ServiceRegistryClient {
   }
 
   private void retry(RequestContext requestContext, Handler<RestResponse> responseHandler) {
-    LOGGER.warn("invoke service [{}] failed, retry.", requestContext.getUri());
-    requestContext.setIpPort(ipPortManager.getNextAvailableAddress(requestContext.getIpPort()));
+    String oldUri = requestContext.getUri();
+    requestContext.setIpPort(ipPortManager.getAvailableAddress());
+    String newUri = requestContext.getUri();
+    LOGGER.warn("invoke service [{}] failed, retry address [{}].", oldUri, newUri);
     requestContext.incrementRetryTimes();
     restClientUtil.httpDo(requestContext, responseHandler);
   }
diff --git a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/TestIpPortManager.java b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/TestIpPortManager.java
index aa9ba90..11c9b85 100644
--- a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/TestIpPortManager.java
+++ b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/TestIpPortManager.java
@@ -79,32 +79,25 @@ public class TestIpPortManager {
     manager.instanceCacheManager = cacheManager;
     IpPort address1 = manager.getAvailableAddress();
 
-    if (address1.getPort() == 9980) {
-      Assert.assertEquals("127.0.0.1", address1.getHostOrIp());
-      Assert.assertEquals(9980, address1.getPort());
-    } else {
-      Assert.assertEquals("127.0.0.1", address1.getHostOrIp());
-      Assert.assertEquals(9981, address1.getPort());
-    }
+    // test initial
+    Assert.assertEquals("127.0.0.1", address1.getHostOrIp());
+    Assert.assertTrue(address1.getPort() == 9980 || address1.getPort() == 9981);
 
-    IpPort address2 = manager.getNextAvailableAddress(address1);
+    // test getAvailableAddress()
+    IpPort address2 = manager.getAvailableAddress();
+    Assert.assertEquals("127.0.0.1", address2.getHostOrIp());
     if (address1.getPort() == 9980) {
-      Assert.assertEquals("127.0.0.1", address2.getHostOrIp());
       Assert.assertEquals(9981, address2.getPort());
     } else {
-      Assert.assertEquals("127.0.0.1", address2.getHostOrIp());
       Assert.assertEquals(9980, address2.getPort());
     }
 
+    // test getAvailableAddress() when reaching the end
     IpPort address3 = manager.getAvailableAddress();
-    if (address1.getPort() == 9980) {
-      Assert.assertEquals("127.0.0.1", address3.getHostOrIp());
-      Assert.assertEquals(9981, address3.getPort());
-    } else {
-      Assert.assertEquals("127.0.0.1", address3.getHostOrIp());
-      Assert.assertEquals(9980, address3.getPort());
-    }
+    Assert.assertEquals("127.0.0.1", address3.getHostOrIp());
+    Assert.assertEquals(address1.getPort(), address3.getPort());
 
+    // mock endpoint list
     Map<String, List<CacheEndpoint>> addresses = new HashMap<>();
     List<CacheEndpoint> instances = new ArrayList<>();
     instances.add(new CacheEndpoint("http://127.0.0.1:9982", null));
@@ -118,31 +111,24 @@ public class TestIpPortManager {
       }
     };
 
+    // test getAvailableAddress() when auto discovery is disabled
     manager.initAutoDiscovery();  //init result is false at first time
-    IpPort address4 = manager.getNextAvailableAddress(address3);
+    IpPort address4 = manager.getAvailableAddress();
+    Assert.assertEquals("127.0.0.1", address4.getHostOrIp());
     if (address1.getPort() == 9980) {
-      Assert.assertEquals("127.0.0.1", address4.getHostOrIp());
-      Assert.assertEquals(9980, address4.getPort());
-    } else {
-      address4 = manager.getNextAvailableAddress(address1);
-      Assert.assertEquals("127.0.0.1", address4.getHostOrIp());
-      Assert.assertEquals(9980, address4.getPort());
+      address4 = manager.getAvailableAddress();
     }
+    Assert.assertEquals(9980, address4.getPort());
 
-    IpPort address5 = manager.getNextAvailableAddress(address4);
+    // test getAvailable address when auto discovery is enabled
+    manager.setAutoDiscoveryInited(true);
+    IpPort address5 = manager.getAvailableAddress();
     Assert.assertEquals("127.0.0.1", address5.getHostOrIp());
     Assert.assertEquals(9981, address5.getPort());
 
-    manager.setAutoDiscoveryInited(true);
-    IpPort address6 = manager.getNextAvailableAddress(address3);
-    if (address1.getPort() == 9980) {
-      Assert.assertEquals("127.0.0.1", address6.getHostOrIp());
-      Assert.assertEquals(9982, address6.getPort());
-    } else {
-      address6 = manager.getNextAvailableAddress(address1);
-      Assert.assertEquals("127.0.0.1", address6.getHostOrIp());
-      Assert.assertEquals(9982, address6.getPort());
-    }
+    IpPort address6 = manager.getAvailableAddress();
+    Assert.assertEquals("127.0.0.1", address6.getHostOrIp());
+    Assert.assertEquals(9982, address6.getPort());
   }
 
   @Test
diff --git a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java
index 3715572..70bc760 100644
--- a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java
+++ b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java
@@ -37,7 +37,6 @@ import org.mockito.Mockito;
 import io.vertx.core.Handler;
 import io.vertx.core.buffer.Buffer;
 import mockit.Expectations;
-import mockit.Injectable;
 import mockit.Mock;
 import mockit.MockUp;
 import mockit.Mocked;
@@ -169,7 +168,7 @@ public class TestClientHttp {
   @Test
   public void testIpPortManager() {
     IpPortManager oManager = new IpPortManager(ServiceRegistryConfig.INSTANCE);
-    IpPort oIPPort = oManager.getNextAvailableAddress(new IpPort("", 33));
+    IpPort oIPPort = oManager.getAvailableAddress();
     Assert.assertEquals(oIPPort.getHostOrIp(), oManager.getAvailableAddress().getHostOrIp());
   }
 }