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/06 02:11:54 UTC

[GitHub] liubao68 closed pull request #853: [SCB-787]fix ping time gap bug and add configuration to time interval

liubao68 closed pull request #853: [SCB-787]fix ping time gap bug and add configuration to time interval
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/853
 
 
   

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/Configuration.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
index 1059e2ec1..bdc38317f 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
@@ -31,6 +31,10 @@
   //// 2.1 configuration items
   public static final String PROP_ROOT = "servicecomb.loadbalance.";
 
+  public static final String RPOP_SERVER_EXPIRED_IN_SECONDS = "servicecomb.loadbalance.stats.serverExpiredInSeconds";
+
+  public static final String RPOP_TIMER_INTERVAL_IN_MINIS = "servicecomb.loadbalance.stats.timerIntervalInMilis";
+
   public static final String PROP_POLICY = "NFLoadBalancerRuleClassName";
 
   public static final String PROP_RULE_STRATEGY_NAME = "strategy.name";
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 8993d7cbb..1b7be1d77 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
@@ -37,6 +37,7 @@
 import com.google.common.cache.LoadingCache;
 import com.google.common.cache.RemovalListener;
 import com.google.common.cache.RemovalNotification;
+import com.netflix.config.DynamicPropertyFactory;
 
 /**
  *  Add special stats that com.netflix.loadbalancer.LoadBalancerStats not provided
@@ -46,9 +47,11 @@
 
   private final Map<ServiceCombServer, ServiceCombServerStats> pingView = new ConcurrentHashMap<>();
 
-  private int serverExpireInSeconds = 10 * 60;
+  private int serverExpireInSeconds = DynamicPropertyFactory.getInstance()
+      .getIntProperty(Configuration.RPOP_SERVER_EXPIRED_IN_SECONDS, 300).get();
 
-  private long timerIntervalInMilis = 10000;
+  private long timerIntervalInMilis = DynamicPropertyFactory.getInstance()
+      .getLongProperty(Configuration.RPOP_TIMER_INTERVAL_IN_MINIS, 10000).get();
 
   private LoadingCache<ServiceCombServer, ServiceCombServerStats> serverStatsCache;
 
@@ -154,7 +157,7 @@ public void run() {
           while (instances.hasNext()) {
             ServiceCombServer server = instances.next();
             ServiceCombServerStats stats = allServers.get(server);
-            if ((System.currentTimeMillis() - stats.getLastVisitTime() < timerIntervalInMilis) && !ping
+            if ((System.currentTimeMillis() - stats.getLastVisitTime() > timerIntervalInMilis) && !ping
                 .ping(server.getInstance())) {
               LOGGER.info("ping mark server {} failure.", server.getInstance().getInstanceId());
               stats.markFailure();
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 3602977cb..11dfe77bb 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
@@ -23,31 +23,57 @@
 import java.util.concurrent.TimeUnit;
 
 import org.apache.servicecomb.core.Transport;
+import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
 import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
 import org.apache.servicecomb.serviceregistry.cache.CacheEndpoint;
+import org.apache.servicecomb.serviceregistry.consumer.MicroserviceInstancePing;
 import org.junit.Assert;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
+import mockit.Expectations;
 import mockit.Injectable;
+import mockit.Mocked;
 
 public class TestServiceCombLoadBalancerStats {
+  @BeforeClass
+  public static void beforeClass() {
+    // avoid mock
+    ServiceCombLoadBalancerStats.INSTANCE.getClass();
+  }
+
   @Test
-  public void testServiceExpire(@Injectable Transport transport) throws Exception {
+  public void testServiceExpire(@Injectable Transport transport, @Mocked SPIServiceUtils utils, @Injectable
+      MicroserviceInstancePing ping) throws Exception {
+    MicroserviceInstance instance = new MicroserviceInstance();
+    instance.setInstanceId("instance1");
+
+    new Expectations() {
+      {
+        SPIServiceUtils.getPriorityHighestService(MicroserviceInstancePing.class);
+        result = ping;
+        ping.ping(instance);
+        result = false;
+      }
+    };
+
     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);
+    ServiceCombServerStats stats = serviceCombLoadBalancerStats.getServiceCombServerStats(serviceCombServer);
     Assert.assertEquals(serviceCombLoadBalancerStats.getPingView().size(), 1);
     await().atMost(5, TimeUnit.SECONDS)
         .until(() -> {
           return serviceCombLoadBalancerStats.getPingView().size() <= 0;
         });
     Assert.assertEquals(serviceCombLoadBalancerStats.getPingView().size(), 0);
+    System.out.print(stats.getFailedRequests());
+    Assert.assertTrue(stats.getFailedRequests() >= 1);
   }
 
   @Test
@@ -60,19 +86,25 @@ public void testSimpleThread(@Injectable Transport transport) {
     ServiceCombLoadBalancerStats.INSTANCE.markFailure(serviceCombServer);
     ServiceCombLoadBalancerStats.INSTANCE.markFailure(serviceCombServer);
     Assert.assertEquals(
-        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getCountinuousFailureCount(), 2);
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getCountinuousFailureCount(),
+        2);
     ServiceCombLoadBalancerStats.INSTANCE.markSuccess(serviceCombServer);
     Assert.assertEquals(
-        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getCountinuousFailureCount(), 0);
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getCountinuousFailureCount(),
+        0);
     ServiceCombLoadBalancerStats.INSTANCE.markSuccess(serviceCombServer);
     Assert
-        .assertEquals(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getTotalRequests(), 4);
-    Assert.assertEquals(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRate(), 50);
-    Assert.assertEquals(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getSuccessRate(), 50);
+        .assertEquals(
+            ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getTotalRequests(), 4);
+    Assert.assertEquals(
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRate(), 50);
+    Assert.assertEquals(
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getSuccessRate(), 50);
     Assert.assertTrue(
         ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getLastVisitTime() <= System
             .currentTimeMillis()
-            && ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getLastVisitTime() >= time);
+            && ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getLastVisitTime()
+            >= time);
   }
 
   @Test
@@ -96,18 +128,24 @@ public void run() {
       }.start();
     }
     latch.await(30, TimeUnit.SECONDS);
-    Assert.assertEquals(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getTotalRequests(),
+    Assert.assertEquals(
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getTotalRequests(),
         4 * 10);
-    Assert.assertEquals(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRate(), 50);
-    Assert.assertEquals(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getSuccessRate(), 50);
-    Assert.assertEquals(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getSuccessRequests(), 20);
+    Assert.assertEquals(
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRate(), 50);
+    Assert.assertEquals(
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getSuccessRate(), 50);
+    Assert.assertEquals(
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getSuccessRequests(), 20);
     Assert.assertTrue(
         ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getLastVisitTime() <= System
             .currentTimeMillis()
-            && ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getLastVisitTime() >= time);
+            && ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getLastVisitTime()
+            >= time);
 
     // time consuming test for timers, taking about 20 seconds. ping timer will update instance status to failure
-    Assert.assertTrue(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRate() <= 50);
+    Assert.assertTrue(
+        ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRate() <= 50);
     long beginTime = System.currentTimeMillis();
     long rate = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRequests();
     while (rate <= 20 &&
@@ -119,6 +157,8 @@ public void run() {
 
     Assert.assertTrue(System.currentTimeMillis() - beginTime < 30000);
     Assert
-        .assertTrue(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRequests() > 20);
+        .assertTrue(
+            ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer).getFailedRequests()
+                > 20);
   }
 }


 

----------------------------------------------------------------
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