You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2018/08/01 07:37:52 UTC
[GitHub] liubao68 closed pull request #845: [SCB]in small & simple
situations, unavailable server not cleaned
liubao68 closed pull request #845: [SCB]in small & simple situations, unavailable server not cleaned
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/845
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/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombLoadBalancerStats.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombLoadBalancerStats.java
index 24845a774..8993d7cbb 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombLoadBalancerStats.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombLoadBalancerStats.java
@@ -21,6 +21,7 @@
import java.util.Map;
import java.util.Timer;
import java.util.TimerTask;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
@@ -30,6 +31,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -42,58 +44,30 @@
public class ServiceCombLoadBalancerStats {
private final static Logger LOGGER = LoggerFactory.getLogger(ServiceCombLoadBalancerStats.class);
- private static final int SERVERSTATS_EXPIRE_MINUTES = 10;
+ private final Map<ServiceCombServer, ServiceCombServerStats> pingView = new ConcurrentHashMap<>();
- private static final long TIMER_INTERVAL_MILLIS = 10000;
+ private int serverExpireInSeconds = 10 * 60;
- private static final LoadingCache<ServiceCombServer, ServiceCombServerStats> SERVER_STATES_CACHE =
- CacheBuilder.newBuilder()
- .expireAfterAccess(SERVERSTATS_EXPIRE_MINUTES, TimeUnit.MINUTES)
- .removalListener(new RemovalListener<ServiceCombServer, ServiceCombServerStats>() {
- @Override
- public void onRemoval(RemovalNotification<ServiceCombServer, ServiceCombServerStats> notification) {
- LOGGER.info("stats of instance {} removed.", notification.getKey().getInstance().getInstanceId());
- }
- })
- .build(
- new CacheLoader<ServiceCombServer, ServiceCombServerStats>() {
- public ServiceCombServerStats load(ServiceCombServer server) {
- return new ServiceCombServerStats();
- }
- });
+ private long timerIntervalInMilis = 10000;
- public static final ServiceCombLoadBalancerStats INSTANCE = new ServiceCombLoadBalancerStats();
+ private LoadingCache<ServiceCombServer, ServiceCombServerStats> serverStatsCache;
- private ServiceCombLoadBalancerStats() {
- Timer timer = new Timer("LoadBalancerStatsTimer", true);
- timer.schedule(new TimerTask() {
- private MicroserviceInstancePing ping = SPIServiceUtils.getPriorityHighestService(MicroserviceInstancePing.class);
+ public static final ServiceCombLoadBalancerStats INSTANCE;
- @Override
- public void run() {
- try {
- Map<ServiceCombServer, ServiceCombServerStats> allServers = SERVER_STATES_CACHE.asMap();
- Iterator<ServiceCombServer> instances = allServers.keySet().iterator();
- while (instances.hasNext()) {
- ServiceCombServer server = instances.next();
- // will not cause reload
- ServiceCombServerStats stats = allServers.get(server);
- if ((System.currentTimeMillis() - stats.getLastVisitTime() < TIMER_INTERVAL_MILLIS) && !ping
- .ping(server.getInstance())) {
- LOGGER.info("ping mark server {} failure.", server.getInstance().getInstanceId());
- markFailure(server);
- }
- }
- } catch (Throwable e) {
- LOGGER.warn("LoadBalancerStatsTimer error.", e);
- }
- }
- }, TIMER_INTERVAL_MILLIS, TIMER_INTERVAL_MILLIS);
+ static {
+ INSTANCE = new ServiceCombLoadBalancerStats();
+ INSTANCE.init();
+ }
+
+ /**
+ * Should be singleton, use it only for testing
+ */
+ ServiceCombLoadBalancerStats() {
}
public void markIsolated(ServiceCombServer server, boolean isolated) {
try {
- SERVER_STATES_CACHE.get(server).markIsolated(isolated);
+ serverStatsCache.get(server).markIsolated(isolated);
} catch (ExecutionException e) {
LOGGER.error("Not expected to happen, maybe a bug.", e);
}
@@ -101,7 +75,7 @@ public void markIsolated(ServiceCombServer server, boolean isolated) {
public void markSuccess(ServiceCombServer server) {
try {
- SERVER_STATES_CACHE.get(server).markSuccess();
+ serverStatsCache.get(server).markSuccess();
} catch (ExecutionException e) {
LOGGER.error("Not expected to happen, maybe a bug.", e);
}
@@ -109,7 +83,7 @@ public void markSuccess(ServiceCombServer server) {
public void markFailure(ServiceCombServer server) {
try {
- SERVER_STATES_CACHE.get(server).markFailure();
+ serverStatsCache.get(server).markFailure();
} catch (ExecutionException e) {
LOGGER.error("Not expected to happen, maybe a bug.", e);
}
@@ -117,7 +91,7 @@ public void markFailure(ServiceCombServer server) {
public ServiceCombServerStats getServiceCombServerStats(ServiceCombServer server) {
try {
- return SERVER_STATES_CACHE.get(server);
+ return serverStatsCache.get(server);
} catch (ExecutionException e) {
LOGGER.error("Not expected to happen, maybe a bug.", e);
return null;
@@ -125,12 +99,73 @@ public ServiceCombServerStats getServiceCombServerStats(ServiceCombServer server
}
public ServiceCombServer getServiceCombServer(MicroserviceInstance instance) {
- for (ServiceCombServer server : SERVER_STATES_CACHE.asMap().keySet()) {
+ for (ServiceCombServer server : serverStatsCache.asMap().keySet()) {
if (server.getInstance().equals(instance)) {
return server;
}
}
return null;
}
+
+ @VisibleForTesting
+ void setServerExpireInSeconds(int sec) {
+ this.serverExpireInSeconds = sec;
+ }
+
+ @VisibleForTesting
+ void setTimerIntervalInMilis(int milis) {
+ this.timerIntervalInMilis = milis;
+ }
+
+ @VisibleForTesting
+ Map<ServiceCombServer, ServiceCombServerStats> getPingView() {
+ return this.pingView;
+ }
+
+ void init() {
+ serverStatsCache =
+ CacheBuilder.newBuilder()
+ .expireAfterAccess(serverExpireInSeconds, TimeUnit.SECONDS)
+ .removalListener(new RemovalListener<ServiceCombServer, ServiceCombServerStats>() {
+ @Override
+ public void onRemoval(RemovalNotification<ServiceCombServer, ServiceCombServerStats> notification) {
+ LOGGER.info("stats of instance {} removed.", notification.getKey().getInstance().getInstanceId());
+ pingView.remove(notification.getKey());
+ }
+ })
+ .build(
+ new CacheLoader<ServiceCombServer, ServiceCombServerStats>() {
+ public ServiceCombServerStats load(ServiceCombServer server) {
+ ServiceCombServerStats stats = new ServiceCombServerStats();
+ pingView.put(server, stats);
+ return stats;
+ }
+ });
+
+ Timer timer = new Timer("LoadBalancerStatsTimer", true);
+ timer.schedule(new TimerTask() {
+ private MicroserviceInstancePing ping = SPIServiceUtils.getPriorityHighestService(MicroserviceInstancePing.class);
+
+ @Override
+ public void run() {
+ try {
+ Map<ServiceCombServer, ServiceCombServerStats> allServers = pingView;
+ Iterator<ServiceCombServer> instances = allServers.keySet().iterator();
+ while (instances.hasNext()) {
+ ServiceCombServer server = instances.next();
+ ServiceCombServerStats stats = allServers.get(server);
+ if ((System.currentTimeMillis() - stats.getLastVisitTime() < timerIntervalInMilis) && !ping
+ .ping(server.getInstance())) {
+ LOGGER.info("ping mark server {} failure.", server.getInstance().getInstanceId());
+ stats.markFailure();
+ }
+ }
+ serverStatsCache.cleanUp();
+ } catch (Throwable e) {
+ LOGGER.warn("LoadBalancerStatsTimer error.", e);
+ }
+ }
+ }, timerIntervalInMilis, timerIntervalInMilis);
+ }
}
diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombLoadBalancerStats.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombLoadBalancerStats.java
index 3a494d305..3602977cb 100644
--- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombLoadBalancerStats.java
+++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestServiceCombLoadBalancerStats.java
@@ -17,6 +17,8 @@
package org.apache.servicecomb.loadbalance;
+import static org.awaitility.Awaitility.await;
+
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@@ -29,6 +31,25 @@
import mockit.Injectable;
public class TestServiceCombLoadBalancerStats {
+ @Test
+ public void testServiceExpire(@Injectable Transport transport) throws Exception {
+ ServiceCombLoadBalancerStats serviceCombLoadBalancerStats = new ServiceCombLoadBalancerStats();
+ serviceCombLoadBalancerStats.setServerExpireInSeconds(2);
+ serviceCombLoadBalancerStats.setTimerIntervalInMilis(500);
+ serviceCombLoadBalancerStats.init();
+ MicroserviceInstance instance = new MicroserviceInstance();
+ instance.setInstanceId("instance1");
+ ServiceCombServer serviceCombServer = new ServiceCombServer(transport,
+ new CacheEndpoint("rest://localhost:8080", instance));
+ serviceCombLoadBalancerStats.markSuccess(serviceCombServer);
+ Assert.assertEquals(serviceCombLoadBalancerStats.getPingView().size(), 1);
+ await().atMost(5, TimeUnit.SECONDS)
+ .until(() -> {
+ return serviceCombLoadBalancerStats.getPingView().size() <= 0;
+ });
+ Assert.assertEquals(serviceCombLoadBalancerStats.getPingView().size(), 0);
+ }
+
@Test
public void testSimpleThread(@Injectable Transport transport) {
long time = System.currentTimeMillis();
----------------------------------------------------------------
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