You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@servicecomb.apache.org by GitBox <gi...@apache.org> on 2018/03/15 11:02:52 UTC
[GitHub] liubao68 closed pull request #595: [SCB-398]fix config cc do not select other instances when error happens
liubao68 closed pull request #595: [SCB-398]fix config cc do not select other instances when error happens
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/595
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/demo/demo-springmvc/springmvc-server/pom.xml b/demo/demo-springmvc/springmvc-server/pom.xml
index f3eaeb019..e6dea9c7d 100644
--- a/demo/demo-springmvc/springmvc-server/pom.xml
+++ b/demo/demo-springmvc/springmvc-server/pom.xml
@@ -43,6 +43,10 @@
<artifactId>metrics-prometheus</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.servicecomb</groupId>
+ <artifactId>config-cc</artifactId>
+ </dependency>
</dependencies>
<properties>
diff --git a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
index 9b0abbb84..be64c9e6c 100644
--- a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
+++ b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
@@ -44,6 +44,12 @@ cse:
interval: 10
watch: false
autodiscovery: true
+ config:
+ client:
+ serverUri: http://127.0.0.1:30103,http://127.0.0.2:30103
+ refreshMode: 0
+ refresh_interval: 5000
+
rest:
address: 0.0.0.0:8080?sslEnabled=true
server:
diff --git a/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/ConfigCenterClient.java b/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/ConfigCenterClient.java
index 0ca5668e9..02684913a 100644
--- a/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/ConfigCenterClient.java
+++ b/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/ConfigCenterClient.java
@@ -100,7 +100,7 @@
private String serviceName = CONFIG_CENTER_CONFIG.getServiceName();
- private List<String> serverUri = CONFIG_CENTER_CONFIG.getServerUri();
+ private MemberDiscovery memberDiscovery = new MemberDiscovery(CONFIG_CENTER_CONFIG.getServerUri());
private ConfigCenterConfigurationSourceImpl.UpdateHandler updateHandler;
@@ -126,9 +126,8 @@ public void connectServer() {
} catch (InterruptedException e) {
throw new IllegalStateException(e);
}
- MemberDiscovery memberdis = new MemberDiscovery(serverUri);
- refreshMembers(memberdis);
- EXECUTOR.scheduleWithFixedDelay(new ConfigRefresh(parseConfigUtils, memberdis),
+ refreshMembers(memberDiscovery);
+ EXECUTOR.scheduleWithFixedDelay(new ConfigRefresh(parseConfigUtils, memberDiscovery),
firstRefreshInterval,
refreshInterval,
TimeUnit.MILLISECONDS);
@@ -183,7 +182,7 @@ private HttpClientOptions createHttpClientOptions() {
httpClientOptions.setProxyOptions(proxy);
}
httpClientOptions.setConnectTimeout(CONFIG_CENTER_CONFIG.getConnectionTimeout());
- if (serverUri.get(0).toLowerCase().startsWith("https")) {
+ if (this.memberDiscovery.getConfigServer().toLowerCase().startsWith("https")) {
LOGGER.debug("config center client performs requests over TLS");
SSLOptionFactory factory = SSLOptionFactory.createSSLOptionFactory(SSL_KEY,
ConfigCenterConfig.INSTANCE.getConcurrentCompositeConfiguration());
@@ -280,7 +279,7 @@ public void doWatch(String configCenter)
waiter.countDown();
},
e -> {
- LOGGER.error("watcher connect to config center {} failed: {}", serverUri, e.getMessage());
+ LOGGER.error("watcher connect to config center {} refresh port {} failed. Error message is [{}]", configCenter, refreshPort, e.getMessage());
waiter.countDown();
});
});
@@ -326,15 +325,15 @@ public void refreshConfig(String configcenter) {
EventManager.post(new ConnSuccEvent());
} catch (IOException e) {
EventManager.post(new ConnFailEvent("config refresh result parse fail " + e.getMessage()));
- LOGGER.error("config refresh result parse fail", e);
+ LOGGER.error("Config refresh from {} failed. Error message is [{}].", configcenter, e.getMessage());
}
});
} else {
rsp.bodyHandler(buf -> {
- LOGGER.error("fetch config fail: " + buf);
+ LOGGER.error("Server error message is [{}].", buf);
});
EventManager.post(new ConnFailEvent("fetch config fail"));
- LOGGER.error("fetch config fail");
+ LOGGER.error("Config refresh from {} failed.", configcenter);
}
});
Map<String, String> headers = new HashMap<>();
@@ -349,7 +348,8 @@ public void refreshConfig(String configcenter) {
headers,
null))));
request.exceptionHandler(e -> {
- LOGGER.error("config refresh fail {}", e.getMessage());
+ EventManager.post(new ConnFailEvent("fetch config fail"));
+ LOGGER.error("Config refresh from {} failed. Error message is [{}].", configcenter, e.getMessage());
});
request.end();
});
diff --git a/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/MemberDiscovery.java b/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/MemberDiscovery.java
index 88b782d7b..6db80e6f0 100644
--- a/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/MemberDiscovery.java
+++ b/dynamic-config/config-cc/src/main/java/org/apache/servicecomb/config/client/MemberDiscovery.java
@@ -20,10 +20,14 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.servicecomb.foundation.common.event.EventManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.eventbus.Subscribe;
+
import io.vertx.core.json.JsonObject;
/**
@@ -37,15 +41,27 @@
private List<String> configServerAddresses = new ArrayList<>();
+ private AtomicInteger counter = new AtomicInteger(0);
+
public MemberDiscovery(List<String> configCenterUri) {
if (configCenterUri != null && !configCenterUri.isEmpty()) {
configServerAddresses.addAll(configCenterUri);
}
Collections.shuffle(configServerAddresses);
+ EventManager.register(this);
}
public String getConfigServer() {
- return configServerAddresses.get(0);
+ if(configServerAddresses.isEmpty()) {
+ throw new IllegalStateException("Config center address is not available.");
+ }
+ int index = Math.abs(counter.get() % configServerAddresses.size());
+ return configServerAddresses.get(index);
+ }
+
+ @Subscribe
+ public void onConnFailEvent(ConnFailEvent e) {
+ counter.incrementAndGet();
}
public void refreshMembers(JsonObject members) {
diff --git a/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestMemberDiscovery.java b/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestMemberDiscovery.java
index 296134f9d..f05cd58cc 100644
--- a/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestMemberDiscovery.java
+++ b/dynamic-config/config-cc/src/test/java/org/apache/servicecomb/config/client/TestMemberDiscovery.java
@@ -19,7 +19,10 @@
import static org.junit.Assert.assertNotNull;
+import java.util.Arrays;
+
import org.apache.servicecomb.config.ConfigUtil;
+import org.apache.servicecomb.foundation.common.event.EventManager;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -38,6 +41,32 @@ public void testGetConfigServerURIs() {
assertNotNull(dc.getConfigServer());
}
+ @Test
+ public void testServerChange() {
+ MemberDiscovery dc = new MemberDiscovery(Arrays.asList("http://127.0.0.1:30103", "http://127.0.0.1:30104"));
+ String server1 = dc.getConfigServer();
+ EventManager.post(new ConnFailEvent("connect failed."));
+ String server2 = dc.getConfigServer();
+ Assert.assertNotEquals(server1, server2);
+ EventManager.post(new ConnFailEvent("connect failed."));
+ server2 = dc.getConfigServer();
+ Assert.assertEquals(server1, server2);
+
+ dc.refreshMembers(new JsonObject(
+ "{\"instances\":"
+ + "[{\"status\":\"UP\",\"endpoints\":[\"rest://0.0.0.0:30109\"],\"hostName\":\"125292-0.0.0.0\",\"serviceName\":\"configServer\",\"isHttps\":false}"
+ + ",{\"status\":\"UP\",\"endpoints\":[\"rest://0.0.0.0:30108\"],\"hostName\":\"125293-0.0.0.0\",\"serviceName\":\"configServer\",\"isHttps\":false}"
+ + "]}"));
+ server1 = dc.getConfigServer();
+ Assert.assertEquals(server1, "http://0.0.0.0:30109");
+ EventManager.post(new ConnFailEvent("connect failed."));
+ server2 = dc.getConfigServer();
+ Assert.assertNotEquals(server1, server2);
+ EventManager.post(new ConnFailEvent("connect failed."));
+ server2 = dc.getConfigServer();
+ Assert.assertEquals(server1, server2);
+ }
+
@Test
public void testDiscoverywithHttp() {
MemberDiscovery discoveryClient1 = new MemberDiscovery(null);
----------------------------------------------------------------
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