From 072d1fadb63c51334aa2c4fc90571a8ec11e0323 Mon Sep 17 00:00:00 2001 From: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:09:41 +0530 Subject: [PATCH] Fix errant GTID detection in VTOrc to also work with a replica not connected to any primary (#17267) Signed-off-by: Manan Gupta --- go/test/endtoend/vtorc/general/vtorc_test.go | 58 +++++++++ go/vt/vtorc/inst/instance_dao.go | 82 ++++++++----- go/vt/vtorc/inst/instance_dao_test.go | 118 +++++++++++++++++++ 3 files changed, 229 insertions(+), 29 deletions(-) diff --git a/go/test/endtoend/vtorc/general/vtorc_test.go b/go/test/endtoend/vtorc/general/vtorc_test.go index 329601deb0c..2ec2fd4b0ae 100644 --- a/go/test/endtoend/vtorc/general/vtorc_test.go +++ b/go/test/endtoend/vtorc/general/vtorc_test.go @@ -62,6 +62,64 @@ func TestPrimaryElection(t *testing.T) { require.Len(t, res.Rows, 1, "There should only be 1 primary tablet which was elected") } +// TestErrantGTIDOnPreviousPrimary tests that VTOrc is able to detect errant GTIDs on a previously demoted primary +// if it has an errant GTID. +func TestErrantGTIDOnPreviousPrimary(t *testing.T) { + defer utils.PrintVTOrcLogsOnFailure(t, clusterInfo.ClusterInstance) + defer cluster.PanicHandler(t) + utils.SetupVttabletsAndVTOrcs(t, clusterInfo, 3, 0, []string{"--change-tablets-with-errant-gtid-to-drained"}, cluster.VTOrcConfiguration{}, 1, "") + keyspace := &clusterInfo.ClusterInstance.Keyspaces[0] + shard0 := &keyspace.Shards[0] + + // find primary from topo + curPrimary := utils.ShardPrimaryTablet(t, clusterInfo, keyspace, shard0) + assert.NotNil(t, curPrimary, "should have elected a primary") + vtOrcProcess := clusterInfo.ClusterInstance.VTOrcProcesses[0] + utils.WaitForSuccessfulRecoveryCount(t, vtOrcProcess, logic.ElectNewPrimaryRecoveryName, 1) + utils.WaitForSuccessfulPRSCount(t, vtOrcProcess, keyspace.Name, shard0.Name, 1) + + var replica, otherReplica *cluster.Vttablet + for _, tablet := range shard0.Vttablets { + // we know we have only two tablets, so the "other" one must be the new primary + if tablet.Alias != curPrimary.Alias { + if replica == nil { + replica = tablet + } else { + otherReplica = tablet + } + } + } + require.NotNil(t, replica, "should be able to find a replica") + require.NotNil(t, otherReplica, "should be able to find 2nd replica") + utils.CheckReplication(t, clusterInfo, curPrimary, []*cluster.Vttablet{replica, otherReplica}, 15*time.Second) + + // Disable global recoveries for the cluster. + vtOrcProcess.DisableGlobalRecoveries(t) + + // Run PRS to promote a different replica. + output, err := clusterInfo.ClusterInstance.VtctldClientProcess.ExecuteCommandWithOutput( + "PlannedReparentShard", + fmt.Sprintf("%s/%s", keyspace.Name, shard0.Name), + "--new-primary", replica.Alias) + require.NoError(t, err, "error in PlannedReparentShard output - %s", output) + + // Stop replicatin on the previous primary to simulate it not reparenting properly. + // Also insert an errant GTID on the previous primary. + err = utils.RunSQLs(t, []string{ + "STOP REPLICA", + "RESET REPLICA ALL", + "set global read_only=OFF", + "insert into vt_ks.vt_insert_test(id, msg) values (10173, 'test 178342')", + }, curPrimary, "") + require.NoError(t, err) + + // Wait for VTOrc to detect the errant GTID and change the tablet to a drained type. + vtOrcProcess.EnableGlobalRecoveries(t) + + // Wait for the tablet to be drained. + utils.WaitForTabletType(t, curPrimary, "drained") +} + // Cases to test: // 1. create cluster with 1 replica and 1 rdonly, let orc choose primary // verify rdonly is not elected, only replica diff --git a/go/vt/vtorc/inst/instance_dao.go b/go/vt/vtorc/inst/instance_dao.go index bd4438dd05f..bd437d1ddac 100644 --- a/go/vt/vtorc/inst/instance_dao.go +++ b/go/vt/vtorc/inst/instance_dao.go @@ -357,35 +357,7 @@ Cleanup: // Add replication group ancestry UUID as well. Otherwise, VTOrc thinks there are errant GTIDs in group // members and its replicas, even though they are not. instance.AncestryUUID = strings.Trim(instance.AncestryUUID, ",") - 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, - // such that the replica may _seems_ to have more entries than the primary, when in fact - // it's just that the primary's probing is stale. - redactedExecutedGtidSet, _ := NewOracleGtidSet(instance.ExecutedGtidSet) - for _, uuid := range strings.Split(instance.AncestryUUID, ",") { - if uuid != instance.ServerUUID { - redactedExecutedGtidSet.RemoveUUID(uuid) - } - if instance.IsCoPrimary && uuid == instance.ServerUUID { - // If this is a co-primary, then this server is likely to show its own generated GTIDs as errant, - // because its co-primary has not applied them yet - redactedExecutedGtidSet.RemoveUUID(uuid) - } - } - // Avoid querying the database if there's no point: - if !redactedExecutedGtidSet.IsEmpty() { - redactedPrimaryExecutedGtidSet, _ := NewOracleGtidSet(instance.primaryExecutedGtidSet) - redactedPrimaryExecutedGtidSet.RemoveUUID(instance.SourceUUID) - - instance.GtidErrant, err = replication.Subtract(redactedExecutedGtidSet.String(), redactedPrimaryExecutedGtidSet.String()) - if err == nil { - var gtidCount int64 - gtidCount, err = replication.GTIDCount(instance.GtidErrant) - currentErrantGTIDCount.Set(tabletAlias, gtidCount) - } - } - } + err = detectErrantGTIDs(tabletAlias, instance, tablet) } latency.Stop("instance") @@ -412,6 +384,58 @@ Cleanup: return nil, err } +// detectErrantGTIDs detects the errant GTIDs on an instance. +func detectErrantGTIDs(tabletAlias string, instance *Instance, tablet *topodatapb.Tablet) (err error) { + // 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. + if instance.primaryExecutedGtidSet == "" && instance.SourceHost == "" { + var primaryInstance *Instance + primaryAlias, _, _ := ReadShardPrimaryInformation(tablet.Keyspace, tablet.Shard) + if primaryAlias != "" { + primaryInstance, _, _ = ReadInstance(primaryAlias) + } + // Only run errant GTID detection, if we are sure that the data read of the current primary + // is up-to-date enough to reflect that it has been promoted. This is needed to prevent + // flagging incorrect errant GTIDs. If we were to use old data, we could have some GTIDs + // accepted by the old primary (this tablet) that don't show in the new primary's set. + if primaryInstance != nil { + if primaryInstance.SourceHost == "" { + instance.primaryExecutedGtidSet = primaryInstance.ExecutedGtidSet + } + } + } + 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, + // such that the replica may _seems_ to have more entries than the primary, when in fact + // it's just that the primary's probing is stale. + redactedExecutedGtidSet, _ := NewOracleGtidSet(instance.ExecutedGtidSet) + for _, uuid := range strings.Split(instance.AncestryUUID, ",") { + if uuid != instance.ServerUUID { + redactedExecutedGtidSet.RemoveUUID(uuid) + } + if instance.IsCoPrimary && uuid == instance.ServerUUID { + // If this is a co-primary, then this server is likely to show its own generated GTIDs as errant, + // because its co-primary has not applied them yet + redactedExecutedGtidSet.RemoveUUID(uuid) + } + } + // Avoid querying the database if there's no point: + if !redactedExecutedGtidSet.IsEmpty() { + redactedPrimaryExecutedGtidSet, _ := NewOracleGtidSet(instance.primaryExecutedGtidSet) + redactedPrimaryExecutedGtidSet.RemoveUUID(instance.SourceUUID) + + instance.GtidErrant, err = replication.Subtract(redactedExecutedGtidSet.String(), redactedPrimaryExecutedGtidSet.String()) + if err == nil { + var gtidCount int64 + gtidCount, err = replication.GTIDCount(instance.GtidErrant) + currentErrantGTIDCount.Set(tabletAlias, gtidCount) + } + } + } + return err +} + // getKeyspaceShardName returns a single string having both the keyspace and shard func getKeyspaceShardName(keyspace, shard string) string { return fmt.Sprintf("%v:%v", keyspace, shard) diff --git a/go/vt/vtorc/inst/instance_dao_test.go b/go/vt/vtorc/inst/instance_dao_test.go index 2416c1abb90..cb2064d10c0 100644 --- a/go/vt/vtorc/inst/instance_dao_test.go +++ b/go/vt/vtorc/inst/instance_dao_test.go @@ -14,6 +14,7 @@ import ( "vitess.io/vitess/go/vt/external/golib/sqlutils" "vitess.io/vitess/go/vt/log" topodatapb "vitess.io/vitess/go/vt/proto/topodata" + "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/topo/topoproto" "vitess.io/vitess/go/vt/vtorc/config" "vitess.io/vitess/go/vt/vtorc/db" @@ -773,3 +774,120 @@ func TestExpireTableData(t *testing.T) { }) } } + +func TestDetectErrantGTIDs(t *testing.T) { + tests := []struct { + name string + instance *Instance + primaryInstance *Instance + wantErr bool + wantErrantGTID string + }{ + { + name: "No errant GTIDs", + instance: &Instance{ + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10539,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34", + primaryExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10591,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34", + AncestryUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff,230ea8ea-81e3-11e4-972a-e25ec4bd140a", + ServerUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + SourceUUID: "230ea8ea-81e3-11e4-972a-e25ec4bd140a", + }, + }, { + name: "Errant GTIDs on replica", + instance: &Instance{ + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10539,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34,316d193c-70e5-11e5-adb2-ecf4bb2262ff:34", + primaryExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10591,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34", + AncestryUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff,230ea8ea-81e3-11e4-972a-e25ec4bd140a", + ServerUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + SourceUUID: "230ea8ea-81e3-11e4-972a-e25ec4bd140a", + }, + wantErrantGTID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff:34", + }, + { + name: "No errant GTIDs on old primary", + instance: &Instance{ + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10539,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34,316d193c-70e5-11e5-adb2-ecf4bb2262ff:1-341", + AncestryUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + ServerUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + }, + primaryInstance: &Instance{ + SourceHost: "", + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10589,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34,316d193c-70e5-11e5-adb2-ecf4bb2262ff:1-341", + }, + }, + { + name: "Errant GTIDs on old primary", + instance: &Instance{ + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10539,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34,316d193c-70e5-11e5-adb2-ecf4bb2262ff:1-342", + AncestryUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + ServerUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + }, + primaryInstance: &Instance{ + SourceHost: "", + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10589,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34,316d193c-70e5-11e5-adb2-ecf4bb2262ff:1-341", + }, + wantErrantGTID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff:342", + }, { + name: "Old information for new primary", + instance: &Instance{ + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10539,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34,316d193c-70e5-11e5-adb2-ecf4bb2262ff:1-342", + AncestryUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + ServerUUID: "316d193c-70e5-11e5-adb2-ecf4bb2262ff", + }, + primaryInstance: &Instance{ + SourceHost: "localhost", + ExecutedGtidSet: "230ea8ea-81e3-11e4-972a-e25ec4bd140a:1-10539,8bc65c84-3fe4-11ed-a912-257f0fcdd6c9:1-34,316d193c-70e5-11e5-adb2-ecf4bb2262ff:1-311", + }, + }, + } + + keyspaceName := "ks" + shardName := "0" + tablet := &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "zone-1", + Uid: 100, + }, + Keyspace: keyspaceName, + Shard: shardName, + } + primaryTablet := &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "zone-1", + Uid: 100, + }, + Keyspace: keyspaceName, + Shard: shardName, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Clear the database after the test. The easiest way to do that is to run all the initialization commands again. + defer func() { + db.ClearVTOrcDatabase() + }() + db.ClearVTOrcDatabase() + + // Save shard record for the primary tablet. + err := SaveShard(topo.NewShardInfo(keyspaceName, shardName, &topodatapb.Shard{ + PrimaryAlias: primaryTablet.Alias, + }, nil)) + require.NoError(t, err) + + if tt.primaryInstance != nil { + tt.primaryInstance.InstanceAlias = topoproto.TabletAliasString(primaryTablet.Alias) + err = SaveTablet(primaryTablet) + require.NoError(t, err) + err = WriteInstance(tt.primaryInstance, true, nil) + require.NoError(t, err) + } + + err = detectErrantGTIDs(topoproto.TabletAliasString(tablet.Alias), tt.instance, tablet) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.EqualValues(t, tt.wantErrantGTID, tt.instance.GtidErrant) + }) + } +}