Add a specialized function to recompute the health of primary tablets.

This ensures that we can never end up in a situation where there is more
than one healthy primary tablet for a given shard.

Signed-off-by: Arthur Schreiber <[email protected]>

go/vt/discovery/healthcheck.go

@@ -37,6 +37,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"hash/crc32"
+	"math"
 	"net/http"
 	"sort"
 	"strings"
@@ -448,10 +449,16 @@ func (hc *HealthCheckImpl) deleteTablet(tablet *topodata.Tablet) {
 				continue
 			}
 			delete(ths, tabletAlias)
-			// delete from healthy list
+
 			healthy, ok := hc.healthy[key]
 			if ok && len(healthy) > 0 {
-				hc.recomputeHealthy(key)
+				if tabletType == topodata.TabletType_PRIMARY {
+					// If tablet type is primary, we should only have one tablet in the healthy list.
+					hc.recomputeHealthyPrimary(key)
+				} else {
+					// Simply recompute the list of healthy tablets for all other tablet types.
+					hc.recomputeHealthy(key)
+				}
 			}
 		}
 	}()
@@ -593,6 +600,18 @@ func (hc *HealthCheckImpl) recomputeHealthy(key KeyspaceShardTabletType) {
 	hc.healthy[key] = FilterStatsByReplicationLag(allArray)
 }
 
+// Recompute the healthy primary tablet for the given key.
+func (hc *HealthCheckImpl) recomputeHealthyPrimary(key KeyspaceShardTabletType) {
+	highestPrimaryTermStartTime := int64(math.MinInt64)
+
+	for _, s := range hc.healthData[key] {
+		if s.PrimaryTermStartTime >= highestPrimaryTermStartTime {
+			highestPrimaryTermStartTime = s.PrimaryTermStartTime
+			hc.healthy[key][0] = s
+		}
+	}
+}
+
 // Subscribe adds a listener. Used by vtgate buffer to learn about primary changes.
 func (hc *HealthCheckImpl) Subscribe() chan *TabletHealth {
 	hc.subMu.Lock()

go/vt/discovery/healthcheck_test.go

@@ -849,6 +849,14 @@ func TestRemoveTablet(t *testing.T) {
 	assert.Empty(t, a, "wrong result, expected empty list")
 }
 
+// When an external primary failover is performed,
+// the demoted primary will advertise itself as a `PRIMARY`
+// tablet until it recognizes that it was demoted,
+// and until all in-flight operations have either finished
+// (successfully or unsuccessfully, see `--shutdown_grace_period` flag).
+//
+// During this time, operations like `RemoveTablet` should not lead
+// to multiple tablets becoming valid targets for `PRIMARY`.
 func TestTabletRemoveDuringExternalReparenting(t *testing.T) {
 	ctx := utils.LeakCheckContext(t)
 
@@ -890,15 +898,15 @@ func TestTabletRemoveDuringExternalReparenting(t *testing.T) {
 	<-resultChan
 	<-resultChan
 
-	firstTabletPromotionTime := time.Now().Unix() - 10
+	firstTabletPrimaryTermStartTimestamp := time.Now().Unix() - 10
 
 	firstTabletHealthStream <- &querypb.StreamHealthResponse{
 		TabletAlias: firstTablet.Alias,
 		Target:      &querypb.Target{Keyspace: "k", Shard: "s", TabletType: topodatapb.TabletType_PRIMARY},
 		Serving:     true,
 
-		PrimaryTermStartTimestamp: firstTabletPromotionTime,
-		RealtimeStats:             &querypb.RealtimeStats{ReplicationLagSeconds: 1, CpuUsage: 0.5},
+		PrimaryTermStartTimestamp: firstTabletPrimaryTermStartTimestamp,
+		RealtimeStats:             &querypb.RealtimeStats{ReplicationLagSeconds: 0, CpuUsage: 0.5},
 	}
 
 	secondTabletHealthStream <- &querypb.StreamHealthResponse{
@@ -923,23 +931,25 @@ func TestTabletRemoveDuringExternalReparenting(t *testing.T) {
 	<-resultChan
 	<-resultChan
 
+	secondTabletPrimaryTermStartTimestamp := time.Now().Unix()
+
 	// Simulate a failover
 	firstTabletHealthStream <- &querypb.StreamHealthResponse{
 		TabletAlias: firstTablet.Alias,
 		Target:      &querypb.Target{Keyspace: "k", Shard: "s", TabletType: topodatapb.TabletType_PRIMARY},
 		Serving:     true,
 
-		PrimaryTermStartTimestamp: 0,
-		RealtimeStats:             &querypb.RealtimeStats{ReplicationLagSeconds: 1, CpuUsage: 0.5},
+		PrimaryTermStartTimestamp: firstTabletPrimaryTermStartTimestamp,
+		RealtimeStats:             &querypb.RealtimeStats{ReplicationLagSeconds: 0, CpuUsage: 0.5},
 	}
 
 	secondTabletHealthStream <- &querypb.StreamHealthResponse{
 		TabletAlias: secondTablet.Alias,
 		Target:      &querypb.Target{Keyspace: "k", Shard: "s", TabletType: topodatapb.TabletType_PRIMARY},
 		Serving:     true,
 
-		PrimaryTermStartTimestamp: time.Now().Unix(),
-		RealtimeStats:             &querypb.RealtimeStats{ReplicationLagSeconds: 1, CpuUsage: 0.5},
+		PrimaryTermStartTimestamp: secondTabletPrimaryTermStartTimestamp,
+		RealtimeStats:             &querypb.RealtimeStats{ReplicationLagSeconds: 0, CpuUsage: 0.5},
 	}
 
 	<-resultChan
@@ -953,7 +963,7 @@ func TestTabletRemoveDuringExternalReparenting(t *testing.T) {
 		Target:               &querypb.Target{Keyspace: "k", Shard: "s", TabletType: topodatapb.TabletType_PRIMARY},
 		Serving:              true,
 		Stats:                &querypb.RealtimeStats{ReplicationLagSeconds: 0, CpuUsage: 0.5},
-		PrimaryTermStartTime: 10,
+		PrimaryTermStartTime: secondTabletPrimaryTermStartTimestamp,
 	}}
 
 	actualTabletStats := hc.GetHealthyTabletStats(&querypb.Target{Keyspace: "k", Shard: "s", TabletType: topodatapb.TabletType_PRIMARY})