Merge pull request #1630 from provenceee/fix-grace

Fix graceful shutdown notify failure
sermant-io · Sep 30, 2024 · a5c184a · a5c184a
2 parents 6253061 + a2e7c14
commit a5c184a
Show file tree

Hide file tree

Showing 17 changed files with 153 additions and 238 deletions.
diff --git a/.github/actions/scenarios/spring/graceful/action.yml b/.github/actions/scenarios/spring/graceful/action.yml
@@ -37,7 +37,6 @@ runs:
         grace.rule.enableGraceShutdown: true
         grace.rule.enableOfflineNotify: true
         grace.rule.warmUpTime: 600
-        grace.rule.upstreamAddressExpiredTime: 600
         servicecomb.service.enableSpringRegister: true
         servicecomb.service.preferIpAddress: true
       # graceful-rest-provider service port 8443 do not change, it special for springCloud Edgware.SR2 test ssl feature.
@@ -67,15 +66,24 @@ runs:
         bash ./sermant-integration-tests/scripts/checkService.sh http://127.0.0.1:8014/testGraceful 60
     - name: test graceful up
       shell: bash
-      run: mvn test -Dsermant.integration.test.type=GRACEFUL -Dgraceful.test.type=up --file sermant-integration-tests/spring-test/pom.xml
-    - name: test graceful down
+      run: |
+        mvn test -Dsermant.integration.test.type=GRACEFUL_FEIGN -Dgraceful.test.type=up --file sermant-integration-tests/spring-test/pom.xml
+        mvn test -Dsermant.integration.test.type=GRACEFUL_REST -Dgraceful.test.type=up --file sermant-integration-tests/spring-test/pom.xml
+    - name: test rest graceful down
       shell: bash
       run: |
-        jps -l | grep graceful-feign-provider | awk '{print $1}' | xargs -n 1 kill
         jps -l | grep graceful-rest-provider | awk '{print $1}' | xargs -n 1 kill
-        echo "========start test graceful======="
+        echo "========start test rest graceful======="
+        jps -l
+        mvn test -Dsermant.integration.test.type=GRACEFUL_REST -Dgraceful.test.type=down --file sermant-integration-tests/spring-test/pom.xml
+    - name: test feign graceful down
+      shell: bash
+      run: |
+        mvn test -Dsermant.integration.test.type=GRACEFUL_FEIGN -Ddown.request.count=100 -Dgraceful.test.type=down --file sermant-integration-tests/spring-test/pom.xml
+        jps -l | grep graceful-feign-provider | awk '{print $1}' | xargs -n 1 kill
+        echo "========start test feign graceful======="
         jps -l
-        mvn test -Dsermant.integration.test.type=GRACEFUL -Dgraceful.test.type=down --file sermant-integration-tests/spring-test/pom.xml
+        mvn test -Dsermant.integration.test.type=GRACEFUL_FEIGN -Dgraceful.test.type=down --file sermant-integration-tests/spring-test/pom.xml
     - name: exit
       if: always()
       uses: ./.github/actions/common/exit

diff --git a/...ing-integration-test/src/test/java/io/sermant/integration/graceful/FeignGracefulTest.java b/...ing-integration-test/src/test/java/io/sermant/integration/graceful/FeignGracefulTest.java
@@ -24,7 +24,7 @@
  * @author zhouss
  * @since 2022-11-15
  */
-@EnabledIfSystemProperty(named = "sermant.integration.test.type", matches = "GRACEFUL")
+@EnabledIfSystemProperty(named = "sermant.integration.test.type", matches = "GRACEFUL_FEIGN")
 public class FeignGracefulTest extends GracefulTest {
     @Override
     protected String getBaseUrl() {

diff --git a/...t/spring-integration-test/src/test/java/io/sermant/integration/graceful/GracefulTest.java b/...t/spring-integration-test/src/test/java/io/sermant/integration/graceful/GracefulTest.java
@@ -51,7 +51,7 @@ public abstract class GracefulTest {
 
     private static final int UP_REQUEST_COUNT = 500;
 
-    private static final int DOWN_REQUEST_COUNT = 1000;
+    private static final String DOWN_REQUEST_COUNT = "1000";
 
     private final String url = getBaseUrl();
 
@@ -104,7 +104,7 @@ public void testGracefulDown() {
             return;
         }
         try {
-            for (int i = 0; i < DOWN_REQUEST_COUNT; i++) {
+            for (int i = 0; i < getDownRequestCount(); i++) {
                 RequestUtils.get(buildUrl("testGraceful"), Collections.emptyMap(), String.class);
             }
         } catch (Exception exception) {
@@ -113,6 +113,10 @@ public void testGracefulDown() {
         }
     }
 
+    private int getDownRequestCount() {
+        return Integer.parseInt(EnvUtils.getEnv("down.request.count", DOWN_REQUEST_COUNT));
+    }
+
     private void statistic(Map<String, Integer> statisticMap) {
         final String port = RequestUtils.get(buildUrl("testGraceful"), Collections.emptyMap(),
                 String.class);

diff --git a/...ring-integration-test/src/test/java/io/sermant/integration/graceful/RestGracefulTest.java b/...ring-integration-test/src/test/java/io/sermant/integration/graceful/RestGracefulTest.java
@@ -24,7 +24,7 @@
  * @author zhouss
  * @since 2022-11-15
  */
-@EnabledIfSystemProperty(named = "sermant.integration.test.type", matches = "GRACEFUL")
+@EnabledIfSystemProperty(named = "sermant.integration.test.type", matches = "GRACEFUL_REST")
 public class RestGracefulTest extends GracefulTest {
     @Override
     protected String getBaseUrl() {

diff --git a/...ervice-registry/registry-common/src/main/java/io/sermant/registry/config/GraceConfig.java b/...ervice-registry/registry-common/src/main/java/io/sermant/registry/config/GraceConfig.java
@@ -114,11 +114,6 @@ public class GraceConfig implements PluginConfig, Cloneable {
      */
     private long upstreamAddressExpiredTime = GraceConstants.UPSTREAM_ADDRESS_DEFAULT_EXPIRED_TIME;
 
-    /**
-     * Cache the wait time of notifying upstream addresses
-     */
-    private long waitNotifyTime = GraceConstants.WAIT_NOTIFY_TIME;
-
     /**
      * Correct the relevant switch attributes according to the aggregation switch,
      * and turn on all functions of elegant online and offline with one click
@@ -267,14 +262,6 @@ public void setUpstreamAddressExpiredTime(long upstreamAddressExpiredTime) {
         this.upstreamAddressExpiredTime = upstreamAddressExpiredTime;
     }
 
-    public long getWaitNotifyTime() {
-        return waitNotifyTime;
-    }
-
-    public void setWaitNotifyTime(long waitNotifyTime) {
-        this.waitNotifyTime = waitNotifyTime;
-    }
-
     /**
      * Check whether the preheating parameter is valid
      *

diff --git a/...gistry/registry-common/src/main/java/io/sermant/registry/config/grace/GraceConstants.java b/...gistry/registry-common/src/main/java/io/sermant/registry/config/grace/GraceConstants.java
@@ -156,11 +156,6 @@ public class GraceConstants {
      */
     public static final long UPSTREAM_ADDRESS_DEFAULT_EXPIRED_TIME = 60L;
 
-    /**
-     * The default wait time of notifying upstream addresses
-     */
-    public static final long WAIT_NOTIFY_TIME = 20L;
-
     /**
      * Maximum port
      */

diff --git a/...y-plugin/src/main/java/io/sermant/registry/config/RegistryConfigSubscribeServiceImpl.java b/...y-plugin/src/main/java/io/sermant/registry/config/RegistryConfigSubscribeServiceImpl.java
@@ -25,7 +25,6 @@
 import io.sermant.core.plugin.subscribe.DefaultGroupConfigSubscriber;
 import io.sermant.core.service.dynamicconfig.common.DynamicConfigEvent;
 import io.sermant.core.service.dynamicconfig.common.DynamicConfigListener;
-import io.sermant.registry.entity.GraceShutdownHook;
 
 import java.util.Locale;
 import java.util.logging.Logger;
@@ -66,9 +65,6 @@ private void fixGrace() {
         // and this configuration has the highest priority
         final GraceConfig graceConfig = PluginConfigManager.getPluginConfig(GraceConfig.class);
         graceConfig.fixGraceSwitch();
-        if (graceConfig.isEnableSpring() && graceConfig.isEnableGraceShutdown()) {
-            Runtime.getRuntime().addShutdownHook(new GraceShutdownHook());
-        }
     }
 
     /**

diff --git a/...cloud-registry-plugin/src/main/java/io/sermant/registry/entity/GraceShutdownBehavior.java b/...cloud-registry-plugin/src/main/java/io/sermant/registry/entity/GraceShutdownBehavior.java
diff --git a/...ing-cloud-registry-plugin/src/main/java/io/sermant/registry/entity/GraceShutdownHook.java b/...ing-cloud-registry-plugin/src/main/java/io/sermant/registry/entity/GraceShutdownHook.java
diff --git a/...ry/spring-cloud-registry-plugin/src/main/java/io/sermant/registry/utils/RefreshUtils.java b/...ry/spring-cloud-registry-plugin/src/main/java/io/sermant/registry/utils/RefreshUtils.java
@@ -96,7 +96,7 @@ private static void refreshWithSpringLb(String serviceName, Collection<String> r
                     "Can not refresh service [%s] instance cache with spring loadbalancer!", curServiceName));
             return;
         }
-        LOGGER.fine(String.format(Locale.ENGLISH,
+        LOGGER.info(String.format(Locale.ENGLISH,
                 "Start refresh target service [%s] spring loadbalancer instance cache!", curServiceName));
         final Optional<Object> cacheOptional = ReflectUtils.invokeMethod(loadBalancerCacheManager, "getCache",
                 new Class[]{String.class}, new Object[]{GraceConstants.SPRING_CACHE_MANAGER_LOADBALANCER_CACHE_NAME});
@@ -112,7 +112,7 @@ private static void refreshWithSpringLb(String serviceName, Collection<String> r
 
     private static void refreshWithRibbon(Object loadbalancer) {
         final Optional<Object> serviceName = ReflectUtils.getFieldValue(loadbalancer, "name");
-        LOGGER.fine(String.format(Locale.ENGLISH,
+        LOGGER.info(String.format(Locale.ENGLISH,
                 "Start refresh target service [%s] ribbon instance cache!", serviceName.orElse("unKnow service")));
         ReflectUtils.invokeMethod(loadbalancer, "updateListOfServers", null, null);
     }

diff --git a/...cloud-registry-plugin/src/test/java/io/sermant/registry/entity/GraceShutdownHookTest.java b/...cloud-registry-plugin/src/test/java/io/sermant/registry/entity/GraceShutdownHookTest.java
diff --git a/...src/test/java/io/sermant/registry/grace/interceptors/SpringWebHandlerInterceptorTest.java b/...src/test/java/io/sermant/registry/grace/interceptors/SpringWebHandlerInterceptorTest.java
@@ -17,13 +17,12 @@
 
 package io.sermant.registry.grace.interceptors;
 
-import io.sermant.registry.config.GraceConfig;
-import io.sermant.registry.config.RegisterConfig;
-import io.sermant.registry.services.GraceService;
-
 import io.sermant.core.plugin.agent.entity.ExecuteContext;
 import io.sermant.core.plugin.config.PluginConfigManager;
 import io.sermant.core.plugin.service.PluginServiceManager;
+import io.sermant.registry.config.GraceConfig;
+import io.sermant.registry.config.RegisterConfig;
+import io.sermant.registry.services.GraceService;
 
 import org.junit.After;
 import org.junit.Assert;

diff --git a/...y-plugin/src/test/java/io/sermant/registry/inject/grace/SpringRequestInterceptorTest.java b/...y-plugin/src/test/java/io/sermant/registry/inject/grace/SpringRequestInterceptorTest.java
@@ -16,14 +16,13 @@
 
 package io.sermant.registry.inject.grace;
 
+import io.sermant.core.plugin.config.PluginConfigManager;
+import io.sermant.core.plugin.service.PluginServiceManager;
 import io.sermant.registry.config.GraceConfig;
 import io.sermant.registry.config.grace.GraceConstants;
 import io.sermant.registry.config.grace.GraceContext;
 import io.sermant.registry.services.GraceService;
 
-import io.sermant.core.plugin.config.PluginConfigManager;
-import io.sermant.core.plugin.service.PluginServiceManager;
-
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;