feat: fix the problem and add tests for it

Signed-off-by: Manan Gupta <[email protected]>

go/vt/vtorc/inst/analysis_dao.go

@@ -294,6 +294,11 @@ func GetReplicationAnalysis(keyspace string, shard string, hints *ReplicationAna
 			return nil
 		}
 
+		// We don't want to run any fixes on any non-replica type tablet.
+		if tablet.Type != topodatapb.TabletType_PRIMARY && !topo.IsReplicaType(tablet.Type) {
+			return nil
+		}
+
 		primaryTablet := &topodatapb.Tablet{}
 		if str := m.GetString("primary_tablet_info"); str != "" {
 			if err := opts.Unmarshal([]byte(str), primaryTablet); err != nil {

go/vt/vtorc/inst/analysis_dao_test.go

@@ -424,6 +424,47 @@ func TestGetReplicationAnalysisDecision(t *testing.T) {
 			keyspaceWanted: "ks",
 			shardWanted:    "0",
 			codeWanted:     ReplicationStopped,
+		}, {
+			name: "No recoveries on drained tablets",
+			info: []*test.InfoForRecoveryAnalysis{{
+				TabletInfo: &topodatapb.Tablet{
+					Alias:         &topodatapb.TabletAlias{Cell: "zon1", Uid: 101},
+					Hostname:      "localhost",
+					Keyspace:      "ks",
+					Shard:         "0",
+					Type:          topodatapb.TabletType_PRIMARY,
+					MysqlHostname: "localhost",
+					MysqlPort:     6708,
+				},
+				DurabilityPolicy:              "none",
+				LastCheckValid:                1,
+				CountReplicas:                 4,
+				CountValidReplicas:            4,
+				CountValidReplicatingReplicas: 3,
+				CountValidOracleGTIDReplicas:  4,
+				CountLoggingReplicas:          2,
+				IsPrimary:                     1,
+			}, {
+				TabletInfo: &topodatapb.Tablet{
+					Alias:         &topodatapb.TabletAlias{Cell: "zon1", Uid: 100},
+					Hostname:      "localhost",
+					Keyspace:      "ks",
+					Shard:         "0",
+					Type:          topodatapb.TabletType_DRAINED,
+					MysqlHostname: "localhost",
+					MysqlPort:     6709,
+				},
+				DurabilityPolicy: "none",
+				PrimaryTabletInfo: &topodatapb.Tablet{
+					Alias: &topodatapb.TabletAlias{Cell: "zon1", Uid: 101},
+				},
+				LastCheckValid:     1,
+				ReadOnly:           1,
+				ReplicationStopped: 1,
+			}},
+			keyspaceWanted: "ks",
+			shardWanted:    "0",
+			codeWanted:     NoProblem,
 		}, {
 			name: "ReplicaMisconfigured",
 			info: []*test.InfoForRecoveryAnalysis{{

go/vt/vtorc/logic/tablet_discovery.go

@@ -204,9 +204,6 @@ func refreshTablets(tablets map[string]*topo.TabletInfo, query string, args []an
 	var wg sync.WaitGroup
 	for _, tabletInfo := range tablets {
 		tablet := tabletInfo.Tablet
-		if tablet.Type != topodatapb.TabletType_PRIMARY && !topo.IsReplicaType(tablet.Type) {
-			continue
-		}
 		tabletAliasString := topoproto.TabletAliasString(tablet.Alias)
 		latestInstances[tabletAliasString] = true
 		old, err := inst.ReadTablet(tabletAliasString)

go/vt/vtorc/logic/tablet_discovery_test.go

@@ -200,6 +200,8 @@ func TestRefreshTabletsInKeyspaceShard(t *testing.T) {
 				return nil
 			})
 			tab100.MysqlPort = 100
+			// We refresh once more to ensure we don't affect the next tests since we've made a change again.
+			refreshTabletsInKeyspaceShard(ctx, keyspace, shard, func(tabletAlias string) {}, false, nil)
 		}()
 		// Let's assume tab100 restarted on a different pod. This would change its tablet hostname and port
 		_, err = ts.UpdateTabletFields(context.Background(), tab100.Alias, func(tablet *topodatapb.Tablet) error {
@@ -212,6 +214,26 @@ func TestRefreshTabletsInKeyspaceShard(t *testing.T) {
 		// Also the old tablet should be forgotten
 		verifyRefreshTabletsInKeyspaceShard(t, false, 1, tablets, nil)
 	})
+
+	t.Run("Replica becomes a drained tablet", func(t *testing.T) {
+		defer func() {
+			_, err = ts.UpdateTabletFields(context.Background(), tab101.Alias, func(tablet *topodatapb.Tablet) error {
+				tablet.Type = topodatapb.TabletType_REPLICA
+				return nil
+			})
+			tab101.Type = topodatapb.TabletType_REPLICA
+			// We refresh once more to ensure we don't affect the next tests since we've made a change again.
+			refreshTabletsInKeyspaceShard(ctx, keyspace, shard, func(tabletAlias string) {}, false, nil)
+		}()
+		// A replica tablet can be converted to drained type if it has an errant GTID.
+		_, err = ts.UpdateTabletFields(context.Background(), tab101.Alias, func(tablet *topodatapb.Tablet) error {
+			tablet.Type = topodatapb.TabletType_DRAINED
+			return nil
+		})
+		tab101.Type = topodatapb.TabletType_DRAINED
+		// We expect 1 tablet to be refreshed since its type has been changed.
+		verifyRefreshTabletsInKeyspaceShard(t, false, 1, tablets, nil)
+	})
 }
 
 func TestShardPrimary(t *testing.T) {