failover triggered mistakenly (#11536)

* Fix failover switch triggered mistakenly and optimise metrics * format code * Fix checkstyle * Fix unit test
alibaba · Mar 7, 2024 · 2f2fc1b · 2f2fc1b
1 parent 4397e8d
commit 2f2fc1b
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 20 deletions.
diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/backups/FailoverReactor.java b/client/src/main/java/com/alibaba/nacos/client/naming/backups/FailoverReactor.java
@@ -26,13 +26,17 @@
 import com.alibaba.nacos.common.spi.NacosServiceLoader;
 import com.alibaba.nacos.common.utils.JacksonUtils;
 import com.alibaba.nacos.common.utils.ThreadUtils;
-import io.micrometer.core.instrument.Gauge;
-import io.micrometer.core.instrument.Meter;
 import io.micrometer.core.instrument.Metrics;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.ImmutableTag;
+import io.micrometer.core.instrument.Gauge;
 
-import java.util.Collection;
-import java.util.HashMap;
 import java.util.Map;
+import java.util.HashMap;
+import java.util.Collection;
+import java.util.List;
+import java.util.ArrayList;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -112,7 +116,7 @@ public void run() {
                     }
 
                     if (failoverMap.size() > 0) {
-                        failoverServiceCntMetrics(failoverMap);
+                        failoverServiceCntMetrics();
                         serviceMap = failoverMap;
                     }
 
@@ -150,6 +154,10 @@ public boolean isFailoverSwitch() {
         return failoverSwitchEnable;
     }
 
+    public boolean isFailoverSwitch(String serviceName) {
+        return failoverSwitchEnable && serviceMap.containsKey(serviceName) && serviceMap.get(serviceName).ipCount() > 0;
+    }
+
     public ServiceInfo getService(String key) {
         ServiceInfo serviceInfo = serviceMap.get(key);
 
@@ -174,15 +182,16 @@ public void shutdown() throws NacosException {
         NAMING_LOGGER.info("{} do shutdown stop", className);
     }
 
-    private void failoverServiceCntMetrics(Map<String, ServiceInfo> failoverMap) {
+    private void failoverServiceCntMetrics() {
         try {
-            for (Map.Entry<String, ServiceInfo> entry : failoverMap.entrySet()) {
+            for (Map.Entry<String, ServiceInfo> entry : serviceMap.entrySet()) {
                 String serviceName = entry.getKey();
-                Gauge register = Gauge
-                        .builder("nacos_naming_client_failover_instances", failoverMap.get(serviceName).ipCount(),
-                                Integer::intValue).tag("service_name", serviceName)
-                        .description("Nacos failover data service count").register(Metrics.globalRegistry);
-                meterMap.put(serviceName, register);
+                List<Tag> tags = new ArrayList<>();
+                tags.add(new ImmutableTag("service_name", serviceName));
+                if (Metrics.globalRegistry.find("nacos_naming_client_failover_instances").tags(tags).gauge() == null) {
+                    Gauge.builder("nacos_naming_client_failover_instances", () -> serviceMap.get(serviceName).ipCount())
+                            .tags(tags).register(Metrics.globalRegistry);
+                }
             }
         } catch (Exception e) {
             NAMING_LOGGER.info("[NA] registerFailoverServiceCnt fail.", e);
@@ -191,12 +200,15 @@ private void failoverServiceCntMetrics(Map<String, ServiceInfo> failoverMap) {
 
     private void failoverServiceCntMetricsClear() {
         try {
-            for (Map.Entry<String, Meter> entry : meterMap.entrySet()) {
-                Metrics.globalRegistry.remove(entry.getValue());
+            for (Map.Entry<String, ServiceInfo> entry : serviceMap.entrySet()) {
+                Gauge gauge = Metrics.globalRegistry.find("nacos_naming_client_failover_instances")
+                        .tag("service_name", entry.getKey()).gauge();
+                if (gauge != null) {
+                    Metrics.globalRegistry.remove(gauge);
+                }
             }
-            meterMap.clear();
         } catch (Exception e) {
             NAMING_LOGGER.info("[NA] registerFailoverServiceCnt fail.", e);
         }
     }
-}
+}
diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java b/client/src/main/java/com/alibaba/nacos/client/naming/cache/ServiceInfoHolder.java
@@ -143,7 +143,7 @@ public ServiceInfo processServiceInfo(ServiceInfo serviceInfo) {
         if (changed) {
             NAMING_LOGGER.info("current ips:({}) service: {} -> {}", serviceInfo.ipCount(), serviceInfo.getKey(),
                     JacksonUtils.toJson(serviceInfo.getHosts()));
-            if (!failoverReactor.isFailoverSwitch()) {
+            if (!failoverReactor.isFailoverSwitch(serviceKey)) {
                 NotifyCenter.publishEvent(
                         new InstancesChangeEvent(notifierEventScope, serviceInfo.getName(), serviceInfo.getGroupName(),
                                 serviceInfo.getClusters(), serviceInfo.getHosts()));

diff --git a/client/src/test/java/com/alibaba/nacos/client/naming/backups/FailoverReactorTest.java b/client/src/test/java/com/alibaba/nacos/client/naming/backups/FailoverReactorTest.java
@@ -135,9 +135,9 @@ public void testRefreshFromEnabledToDisabled()
     @Test
     public void testFailoverServiceCntMetrics()
             throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
-        Method method = FailoverReactor.class.getDeclaredMethod("failoverServiceCntMetrics", Map.class);
+        Method method = FailoverReactor.class.getDeclaredMethod("failoverServiceCntMetrics");
         method.setAccessible(true);
-        method.invoke(failoverReactor, new Object[1]);
+        method.invoke(failoverReactor);
         // No exception
     }
 
@@ -152,4 +152,4 @@ public void testFailoverServiceCntMetricsClear()
         method.invoke(failoverReactor);
         // No exception
     }
-}
+}