[ https://issues.apache.org/jira/browse/SCB-398?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16400230#comment-16400230 ]
ASF GitHub Bot commented on SCB-398: ------------------------------------ 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: us...@infra.apache.org > when configured multiple cc addresses and error happens, will not change an > available one > ----------------------------------------------------------------------------------------- > > Key: SCB-398 > URL: https://issues.apache.org/jira/browse/SCB-398 > Project: Apache ServiceComb > Issue Type: Bug > Reporter: liubao > Assignee: liubao > Priority: Major > > when configured multiple cc addresses and error happens, will not change an > available one -- This message was sent by Atlassian JIRA (v7.6.3#76005)