[GitHub] WillemJiang commented on a change in pull request #486: [SCB-165] Service registry not given proper instances

[GitBox <gi...@apache.org>]

WillemJiang commented on a change in pull request #486: [SCB-165] Service registry not given proper instances
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/486#discussion_r159832477
 
 

 ##########
 File path: service-registry/src/main/java/io/servicecomb/serviceregistry/client/IpPortManager.java
 ##########
 @@ -65,45 +66,47 @@ public IpPortManager(ServiceRegistryConfig serviceRegistryConfig, InstanceCacheM
 
   // we have to do this operation after the first time setup has already done
   public void initAutoDiscovery() {
-    if (this.serviceRegistryConfig.isRegistryAutoDiscovery()) {
-      instanceCache = instanceCacheManager.getOrCreate(REGISTRY_APP_ID,
+    if (!autoDiscoveryInited && this.serviceRegistryConfig.isRegistryAutoDiscovery()) {
+      instanceCacheManager.getOrCreate(REGISTRY_APP_ID,
           REGISTRY_SERVICE_NAME,
           DefinitionConst.VERSION_RULE_LATEST);
+      autoDiscoveryInited = true;
     }
   }
 
-  public IpPort getAvailableAddress(boolean invalidate) {
-    int index;
-    if (invalidate) {
-      index = currentAvailableIndex.incrementAndGet();
-    } else {
-      index = currentAvailableIndex.get();
+  public IpPort getNextAvailableAddress(IpPort failedIpPort) {
+    IpPort current = getAvailableAddress();
+    if (current.equals(failedIpPort)) {
+      currentAvailableIndex.incrementAndGet();
+      current = getAvailableAddress();
     }
 
+    LOGGER.info("Change service center address from {} to {}", failedIpPort.toString(), current.toString());
+    return current;
+  }
+
+  public IpPort getAvailableAddress() {
+    return getAvailableAddress(currentAvailableIndex.get());
+  }
+
+  private IpPort getAvailableAddress(int index) {
     if (index < defaultIpPort.size()) {
-      return returnWithLog(invalidate, defaultIpPort.get(index));
+      return defaultIpPort.get(index);
     }
     List<CacheEndpoint> endpoints = getDiscoveredIpPort();
     if (endpoints == null || (index >= defaultIpPort.size() + endpoints.size())) {
       currentAvailableIndex.set(0);
-      return returnWithLog(invalidate, defaultIpPort.get(0));
+      return defaultIpPort.get(0);
     }
     CacheEndpoint nextEndpoint = endpoints.get(index - defaultIpPort.size());
-    return returnWithLog(invalidate, new URIEndpointObject(nextEndpoint.getEndpoint()));
-  }
-
-  private IpPort returnWithLog(boolean needLog, IpPort result) {
-    if (needLog) {
-      LOGGER.info("Using next service center address {}:{}.", result.getHostOrIp(), result.getPort());
-    }
-    return result;
+    return new URIEndpointObject(nextEndpoint.getEndpoint());
   }
 
   private List<CacheEndpoint> getDiscoveredIpPort() {
-    if (instanceCache == null) {
-      return new ArrayList<>(0);
+    if (!autoDiscoveryInited || !this.serviceRegistryConfig.isRegistryAutoDiscovery()) {
+      return null;
 
 Review comment:
   Is it safe to run null here?  How about return the empty list?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services