[release-24.0] vtorc: reset CurrentErrantGTIDCount gauge when errant GTIDs are resolved (#20259) (#20292)

Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>
Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: vitess-bot[bot]

go/vt/vtorc/inst/instance_dao.go

@@ -406,6 +406,7 @@ Cleanup:
 
 // detectErrantGTIDs detects the errant GTIDs on an instance.
 func detectErrantGTIDs(instance *Instance, tablet *topodatapb.Tablet) (err error) {
+	tabletAliasString := topoproto.TabletAliasString(instance.InstanceAlias)
 	// If the tablet is not replicating from anyone, then it could be the previous primary.
 	// We should check for errant GTIDs by finding the difference with the shard's current primary.
 	primaryAlias, _, err := ReadShardPrimaryInformation(tablet.Keyspace, tablet.Shard)
@@ -416,6 +417,10 @@ func detectErrantGTIDs(instance *Instance, tablet *topodatapb.Tablet) (err error
 	// Check if the current tablet is the primary. If it is, then we don't need to
 	// run errant GTID detection on it.
 	if topoproto.TabletAliasEqual(primaryAlias, instance.InstanceAlias) {
+		// A primary cannot have errant GTIDs relative to itself; clear any
+		// value left over from before this tablet was promoted.
+		instance.GtidErrant = ""
+		currentErrantGTIDCount.Reset(tabletAliasString)
 		return nil
 	}
 
@@ -439,6 +444,7 @@ func detectErrantGTIDs(instance *Instance, tablet *topodatapb.Tablet) (err error
 		}
 	}
 
+	var errantGtidCount int64
 	if instance.ExecutedGtidSet != "" && instance.primaryExecutedGtidSet != "" {
 		// Compare primary & replica GTID sets, but ignore the sets that present the primary's UUID.
 		// This is because vtorc may pool primary and replica at an inconvenient timing,
@@ -469,10 +475,17 @@ func detectErrantGTIDs(instance *Instance, tablet *topodatapb.Tablet) (err error
 			errantGtidSet := redactedExecutedGtidSet.Difference(redactedPrimaryExecutedGtidSet)
 			if !errantGtidSet.Empty() {
 				instance.GtidErrant = errantGtidSet.String()
-				currentErrantGTIDCount.Set(topoproto.TabletAliasString(instance.InstanceAlias), errantGtidSet.Count())
+				errantGtidCount = errantGtidSet.Count()
 			}
 		}
 	}
+	// Always publish the result. Writing 0 / "" here is what allows the
+	// gauge and GtidErrant field to recover after errant GTIDs are
+	// reconciled or a transient race clears.
+	if errantGtidCount == 0 {
+		instance.GtidErrant = ""
+	}
+	currentErrantGTIDCount.Set(tabletAliasString, errantGtidCount)
 	return err
 }
 

go/vt/vtorc/inst/instance_dao_test.go

@@ -1156,3 +1156,63 @@ func TestPrimaryErrantGTIDs(t *testing.T) {
 	require.NoError(t, err)
 	require.EqualValues(t, "", instance.GtidErrant)
 }
+
+// TestErrantGTIDCountGaugeIsResetWhenResolved verifies that the per-tablet
+// CurrentErrantGTIDCount gauge is reset to 0 when errant GTIDs are reconciled
+// on a subsequent poll. Previously, the gauge was only ever written when
+// errant GTIDs were detected, so it stuck at the last positive value forever
+// (https://github.com/vitessio/vitess/issues/20258).
+func TestErrantGTIDCountGaugeIsResetWhenResolved(t *testing.T) {
+	defer func() {
+		db.ClearVTOrcDatabase()
+		currentErrantGTIDCount.ResetAll()
+	}()
+	db.ClearVTOrcDatabase()
+	currentErrantGTIDCount.ResetAll()
+
+	keyspaceName := "ks"
+	shardName := "0"
+	primaryUUID := "230ea8ea-81e3-11e4-972a-e25ec4bd140a"
+	replicaUUID := "316d193c-70e5-11e5-adb2-ecf4bb2262ff"
+
+	primaryTablet := &topodatapb.Tablet{
+		Alias:    &topodatapb.TabletAlias{Cell: "zone-1", Uid: 101},
+		Keyspace: keyspaceName,
+		Shard:    shardName,
+		Type:     topodatapb.TabletType_PRIMARY,
+	}
+	replicaTablet := &topodatapb.Tablet{
+		Alias:    &topodatapb.TabletAlias{Cell: "zone-1", Uid: 100},
+		Keyspace: keyspaceName,
+		Shard:    shardName,
+	}
+	replicaAlias := topoproto.TabletAliasString(replicaTablet.Alias)
+
+	require.NoError(t, SaveShard(topo.NewShardInfo(keyspaceName, shardName, &topodatapb.Shard{
+		PrimaryAlias: primaryTablet.Alias,
+	}, nil)))
+
+	// First poll: the replica has an errant GTID under its own UUID.
+	replicaInstance := &Instance{
+		InstanceAlias:          replicaTablet.Alias,
+		ServerUUID:             replicaUUID,
+		SourceUUID:             primaryUUID,
+		AncestryUUID:           replicaUUID + "," + primaryUUID,
+		ExecutedGtidSet:        primaryUUID + ":1-100," + replicaUUID + ":1",
+		primaryExecutedGtidSet: primaryUUID + ":1-100",
+	}
+	require.NoError(t, detectErrantGTIDs(replicaInstance, replicaTablet))
+	require.Equal(t, replicaUUID+":1", replicaInstance.GtidErrant)
+	require.EqualValues(t, 1, currentErrantGTIDCount.Counts()[replicaAlias],
+		"gauge should be 1 after a single errant GTID is detected")
+
+	// Second poll: the errant GTID has been reconciled (e.g., via the
+	// empty-transaction injection technique on the primary), so the primary's
+	// executed GTID set now also includes the replica UUID range.
+	replicaInstance.primaryExecutedGtidSet = primaryUUID + ":1-100," + replicaUUID + ":1"
+	require.NoError(t, detectErrantGTIDs(replicaInstance, replicaTablet))
+	require.Empty(t, replicaInstance.GtidErrant,
+		"GtidErrant should be cleared on the no-errant path so callers reusing the Instance don't see stale state")
+	require.EqualValues(t, 0, currentErrantGTIDCount.Counts()[replicaAlias],
+		"gauge should be reset to 0 after errant GTIDs are resolved")
+}