From 0092c7c5281e8f6c6db244818a301fbca7092581 Mon Sep 17 00:00:00 2001
From: wangweicugw <wangweicugw@163.com>
Date: Wed, 14 Sep 2022 19:53:02 +0800
Subject: [PATCH] Fixing the issue of 'no valid tablet' caused by the noserving
 master not being offline in a dual-master setup.

---
 .../com/jd/jdbc/discovery/HealthCheck.java    | 13 +++--
 .../jd/jdbc/discovery/HealthCheckTest.java    | 50 +++++++++++++++++++
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/main/java/com/jd/jdbc/discovery/HealthCheck.java b/src/main/java/com/jd/jdbc/discovery/HealthCheck.java
index 3a57e7d..0413f93 100644
--- a/src/main/java/com/jd/jdbc/discovery/HealthCheck.java
+++ b/src/main/java/com/jd/jdbc/discovery/HealthCheck.java
@@ -18,6 +18,7 @@
 
 package com.jd.jdbc.discovery;
 
+import com.jd.jdbc.common.util.CollectionUtils;
 import com.jd.jdbc.common.util.MapUtil;
 import com.jd.jdbc.context.IContext;
 import com.jd.jdbc.monitor.HealthCheckCollector;
@@ -348,7 +349,6 @@ public void replaceTablet(Topodata.Tablet oldTablet, Topodata.Tablet newTablet)
         this.addTablet(newTablet);
     }
 
-    //
     public void updateHealth(final TabletHealthCheck th, final Query.Target preTarget,
                              final boolean trivialUpdate, boolean up) {
         String tabletAlias = TopoProto.tabletAliasString(th.getTablet().getAlias());
@@ -388,8 +388,15 @@ public void updateHealth(final TabletHealthCheck th, final Query.Target preTarge
                         }
                     }
                 } else {
-                    List<TabletHealthCheck> healthCheckList = new ArrayList<>();
-                    this.healthy.put(targetKey, healthCheckList);
+                    List<TabletHealthCheck> tabletHealthChecks = healthy.get(targetKey);
+                    if (CollectionUtils.isNotEmpty(tabletHealthChecks)) {
+                        // isPrimary is true here therefore we should only have 1 tablet in healthy
+                        String aliasString = TopoProto.tabletAliasString(tabletHealthChecks.get(0).getTablet().getAlias());
+                        // Clear healthy list for primary if the existing tablet is down
+                        if (Objects.equals(tabletAlias, aliasString)) {
+                            this.healthy.put(targetKey, new ArrayList<>());
+                        }
+                    }
                 }
             }
             if (!trivialUpdate) {
diff --git a/src/test/java/com/jd/jdbc/discovery/HealthCheckTest.java b/src/test/java/com/jd/jdbc/discovery/HealthCheckTest.java
index 766f5e4..eacdd55 100644
--- a/src/test/java/com/jd/jdbc/discovery/HealthCheckTest.java
+++ b/src/test/java/com/jd/jdbc/discovery/HealthCheckTest.java
@@ -18,6 +18,7 @@
 
 package com.jd.jdbc.discovery;
 
+import com.jd.jdbc.common.util.CollectionUtils;
 import com.jd.jdbc.context.IContext;
 import com.jd.jdbc.context.VtContext;
 import com.jd.jdbc.discovery.TabletHealthCheck.TabletStreamHealthStatus;
@@ -841,6 +842,55 @@ public void testMysqlPort3358to0() throws IOException, InterruptedException {
         printOk();
     }
 
+    @Test
+    public void testDoubleMaster() throws IOException, InterruptedException {
+        printComment("16. double master one no serving");
+        printComment("a. Get Health");
+        HealthCheck hc = getHealthCheck();
+
+        printComment("b. Add a no-serving Tablet");
+
+        // add master tablet
+        MockTablet mockTablet = buildMockTablet("cell", 0, "a", "k", "s", portMap, Topodata.TabletType.MASTER);
+        hc.addTablet(mockTablet.getTablet());
+        // add replica tablet
+        MockTablet mockTablet1 = buildMockTablet("cell", 1, "b", "k", "s", portMap, Topodata.TabletType.REPLICA);
+        hc.addTablet(mockTablet1.getTablet());
+
+        Thread.sleep(200);
+        Assert.assertEquals("Wrong Tablet data", 2, hc.getHealthByAliasCopy().size());
+        Assert.assertEquals("Wrong Healthy Tablet data", 0, hc.getHealthyCopy().size());
+
+        printComment("c. Modify the status of Tablet to serving");
+        sendOnNextMessage(mockTablet, Topodata.TabletType.MASTER, true, 0, 0.5, 0);
+        sendOnNextMessage(mockTablet1, Topodata.TabletType.REPLICA, true, 0, 0.5, 1);
+
+        Thread.sleep(200);
+
+        Assert.assertEquals("Wrong Tablet data", 2, hc.getHealthByAliasCopy().size());
+        Assert.assertEquals("Wrong Healthy Tablet data", 2, hc.getHealthyCopy().size());
+
+        printComment("d. Modify the role of tablet");
+        sendOnNextMessage(mockTablet1, Topodata.TabletType.MASTER, true, 10, 0.5, 0);
+        Thread.sleep(200);
+
+        printComment("e. Modify old master Tablet to no serving");
+        sendOnNextMessage(mockTablet, Topodata.TabletType.MASTER, false, 0, 0.5, 0);
+        Thread.sleep(200);
+
+        Assert.assertEquals("Wrong Tablet data", 2, hc.getHealthByAliasCopy().size());
+        Assert.assertEquals("Wrong Healthy Tablet data", 1, hc.getHealthyCopy().size());
+
+        List<TabletHealthCheck> healthyTabletStats = hc.getHealthyTabletStats(createTarget(Topodata.TabletType.REPLICA));
+        Assert.assertTrue(CollectionUtils.isEmpty(healthyTabletStats));
+
+        healthyTabletStats = hc.getHealthyTabletStats(createTarget(Topodata.TabletType.MASTER));
+        Assert.assertTrue(CollectionUtils.isNotEmpty(healthyTabletStats));
+        Assert.assertEquals(1, healthyTabletStats.size());
+
+        printOk();
+    }
+
     @Test
     public void testHealthyListChecksum() {
         HealthCheck hc = getHealthCheck();