vtorc: use *topoprotopb.TabletAlias for tablet alias (vitessio#18389)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

Author: timvaillancourt

go/test/endtoend/vtorc/api/api_test.go

@@ -85,7 +85,7 @@ func TestAPIEndpoints(t *testing.T) {
 	})
 
 	// Before we disable recoveries, let us wait until VTOrc has fixed all the issues (if any).
-	_, _ = utils.MakeAPICallRetry(t, vtorc, "/api/replication-analysis", func(_ int, response string) bool {
+	_, _ = utils.MakeAPICallRetry(t, vtorc, "/api/detection-analysis", func(i int, response string) bool {
 		return response != "null"
 	})
 
@@ -164,41 +164,41 @@ func TestAPIEndpoints(t *testing.T) {
 		assert.Equal(t, "Global recoveries disabled\n", resp)
 	})
 
-	t.Run("Replication Analysis API", func(t *testing.T) {
+	t.Run("Detection Analysis API", func(t *testing.T) {
 		// use vtctldclient to stop replication
 		_, err := clusterInfo.ClusterInstance.VtctldClientProcess.ExecuteCommandWithOutput("StopReplication", replica.Alias)
 		require.NoError(t, err)
 
 		// We know VTOrc won't fix this since we disabled global recoveries!
 		// Wait until VTOrc picks up on this issue and verify
-		// that we see a not null result on the api/replication-analysis page
-		status, resp := utils.MakeAPICallRetry(t, vtorc, "/api/replication-analysis", func(_ int, response string) bool {
+		// that we see a not null result on the api/detection-analysis page
+		status, resp := utils.MakeAPICallRetry(t, vtorc, "/api/detection-analysis", func(_ int, response string) bool {
 			return response == "null"
 		})
 		assert.Equal(t, 200, status, resp)
 		assert.Contains(t, resp, fmt.Sprintf(`"AnalyzedInstanceAlias": "%s"`, replica.Alias))
 		assert.Contains(t, resp, `"Analysis": "ReplicationStopped"`)
 
 		// Verify that filtering also works in the API as intended
-		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/replication-analysis?keyspace=ks&shard=0")
+		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/detection-analysis?keyspace=ks&shard=0")
 		require.NoError(t, err)
 		assert.Equal(t, 200, status, resp)
 		assert.Contains(t, resp, fmt.Sprintf(`"AnalyzedInstanceAlias": "%s"`, replica.Alias))
 
 		// Verify that filtering by keyspace also works in the API as intended
-		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/replication-analysis?keyspace=ks")
+		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/detection-analysis?keyspace=ks")
 		require.NoError(t, err)
 		assert.Equal(t, 200, status, resp)
 		assert.Contains(t, resp, fmt.Sprintf(`"AnalyzedInstanceAlias": "%s"`, replica.Alias))
 
 		// Check that filtering using keyspace and shard works
-		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/replication-analysis?keyspace=ks&shard=80-")
+		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/detection-analysis?keyspace=ks&shard=80-")
 		require.NoError(t, err)
 		assert.Equal(t, 200, status, resp)
 		assert.Equal(t, "null", resp)
 
 		// Check that filtering using just the shard fails
-		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/replication-analysis?shard=0")
+		status, resp, err = utils.MakeAPICall(t, vtorc, "/api/detection-analysis?shard=0")
 		require.NoError(t, err)
 		assert.Equal(t, 400, status, resp)
 		assert.Equal(t, "Filtering by shard without keyspace isn't supported\n", resp)

go/test/endtoend/vtorc/general/vtorc_test.go

@@ -848,7 +848,7 @@ func TestFullStatusConnectionPooling(t *testing.T) {
 
 	// Wait until VTOrc notices the failure.
 	require.Eventually(t, func() bool {
-		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/replication-analysis")
+		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/detection-analysis")
 		return err == nil && status == 200 && strings.Contains(resp, "UnreachablePrimary")
 	}, 90*time.Second, time.Second, "timed out waiting for UnreachablePrimary analysis")
 
@@ -860,7 +860,7 @@ func TestFullStatusConnectionPooling(t *testing.T) {
 
 	// See that VTOrc eventually reports no errors.
 	require.Eventually(t, func() bool {
-		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/replication-analysis")
+		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/detection-analysis")
 		return err == nil && status == 200 && strings.TrimSpace(resp) == "null"
 	}, 90*time.Second, time.Second, "timed out waiting for replication analysis to clear")
 
@@ -870,7 +870,7 @@ func TestFullStatusConnectionPooling(t *testing.T) {
 
 	// Wait until VTOrc notices the failure.
 	require.Eventually(t, func() bool {
-		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/replication-analysis")
+		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/detection-analysis")
 		return err == nil && status == 200 && strings.Contains(resp, "UnreachablePrimary")
 	}, 90*time.Second, time.Second, "timed out waiting for UnreachablePrimary analysis")
 
@@ -882,7 +882,7 @@ func TestFullStatusConnectionPooling(t *testing.T) {
 
 	// See that VTOrc eventually reports no errors.
 	require.Eventually(t, func() bool {
-		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/replication-analysis")
+		status, resp, err := utils.MakeAPICall(t, vtorc, "/api/detection-analysis")
 		return err == nil && status == 200 && strings.TrimSpace(resp) == "null"
 	}, 90*time.Second, time.Second, "timed out waiting for replication analysis to clear")
 }

go/test/endtoend/vtorc/readtopologyinstance/main_test.go

@@ -26,7 +26,7 @@ import (
 	"vitess.io/vitess/go/test/endtoend/vtorc/utils"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	"vitess.io/vitess/go/vt/servenv"
-
+	"vitess.io/vitess/go/vt/topo/topoproto"
 	"vitess.io/vitess/go/vt/vtorc/config"
 	"vitess.io/vitess/go/vt/vtorc/inst"
 	"vitess.io/vitess/go/vt/vtorc/logic"
@@ -78,13 +78,19 @@ func TestReadTopologyInstanceBufferable(t *testing.T) {
 		}
 	}
 
-	primaryInstance, err := inst.ReadTopologyInstanceBufferable(primary.Alias, nil)
+	primaryAlias, err := topoproto.ParseTabletAlias(primary.Alias)
+	require.NoError(t, err)
+
+	replicaAlias, err := topoproto.ParseTabletAlias(replica.Alias)
+	require.NoError(t, err)
+
+	primaryInstance, err := inst.ReadTopologyInstanceBufferable(primaryAlias, nil)
 	require.NoError(t, err)
 	require.NotNil(t, primaryInstance)
 	assert.Equal(t, utils.Hostname, primaryInstance.Hostname)
 	assert.Equal(t, primary.MySQLPort, primaryInstance.Port)
 	assert.Equal(t, topodatapb.TabletType_PRIMARY, primaryInstance.TabletType)
-	assert.Contains(t, primaryInstance.InstanceAlias, "zone1")
+	assert.Contains(t, primaryInstance.InstanceAlias.String(), "zone1")
 	assert.NotEqual(t, 0, primaryInstance.ServerID)
 	assert.Greater(t, len(primaryInstance.ServerUUID), 10)
 	assert.Regexp(t, "[58].[704].*", primaryInstance.Version)
@@ -131,13 +137,13 @@ func TestReadTopologyInstanceBufferable(t *testing.T) {
 	err = logic.EnableRecovery()
 	require.NoError(t, err)
 
-	replicaInstance, err := inst.ReadTopologyInstanceBufferable(replica.Alias, nil)
+	replicaInstance, err := inst.ReadTopologyInstanceBufferable(replicaAlias, nil)
 	require.NoError(t, err)
 	require.NotNil(t, replicaInstance)
 	assert.Equal(t, utils.Hostname, replicaInstance.Hostname)
 	assert.Equal(t, replica.MySQLPort, replicaInstance.Port)
 	assert.Equal(t, topodatapb.TabletType_REPLICA, replicaInstance.TabletType)
-	assert.Contains(t, replicaInstance.InstanceAlias, "zone1")
+	assert.Contains(t, replicaInstance.InstanceAlias.String(), "zone1")
 	assert.NotEqual(t, 0, replicaInstance.ServerID)
 	assert.Greater(t, len(replicaInstance.ServerUUID), 10)
 	assert.Regexp(t, "[58].[704].*", replicaInstance.Version)

go/vt/vtorc/inst/analysis.go

@@ -93,8 +93,8 @@ type DetectionAnalysisHints struct {
 
 // DetectionAnalysis represents an analysis of a detected problem.
 type DetectionAnalysis struct {
-	AnalyzedInstanceAlias        string
-	AnalyzedInstancePrimaryAlias string
+	AnalyzedInstanceAlias        *topodatapb.TabletAlias
+	AnalyzedInstancePrimaryAlias *topodatapb.TabletAlias
 
 	// TabletType is the tablet's type as seen in the topology.
 	TabletType topodatapb.TabletType

go/vt/vtorc/inst/analysis_dao.go

@@ -52,7 +52,7 @@ func initializeAnalysisDaoPostConfiguration() {
 type clusterAnalysis struct {
 	hasShardWideAction bool
 	totalTablets       int
-	primaryAlias       string
+	primaryAlias       *topodatapb.TabletAlias
 
 	// primaryTimestamp is the most recent primary term start time observed for the shard.
 	primaryTimestamp time.Time
@@ -332,8 +332,8 @@ func GetDetectionAnalysis(keyspace string, shard string, hints *DetectionAnalysi
 
 		a.ShardPrimaryTermTimestamp = m.GetTime("shard_primary_term_timestamp")
 		a.IsPrimary = m.GetBool("is_primary")
-		a.AnalyzedInstanceAlias = topoproto.TabletAliasString(tablet.Alias)
-		a.AnalyzedInstancePrimaryAlias = topoproto.TabletAliasString(primaryTablet.Alias)
+		a.AnalyzedInstanceAlias = tablet.Alias
+		a.AnalyzedInstancePrimaryAlias = primaryTablet.Alias
 		a.AnalyzedInstanceBinlogCoordinates = BinlogCoordinates{
 			LogFile: m.GetString("binary_log_file"),
 			LogPos:  m.GetUint64("binary_log_pos"),
@@ -569,12 +569,13 @@ func postProcessAnalyses(result []*DetectionAnalysis, clusters map[string]*clust
 // auditInstanceAnalysisInChangelog will write down an instance's analysis in the database_instance_analysis_changelog table.
 // To not repeat recurring analysis code, the database_instance_last_analysis table is used, so that only changes to
 // analysis codes are written.
-func auditInstanceAnalysisInChangelog(tabletAlias string, analysisCode AnalysisCode) error {
-	if lastWrittenAnalysis, found := recentInstantAnalysis.Get(tabletAlias); found {
+func auditInstanceAnalysisInChangelog(tabletAlias *topodatapb.TabletAlias, analysisCode AnalysisCode) error {
+	tabletAliasString := topoproto.TabletAliasString(tabletAlias)
+	if lastWrittenAnalysis, found := recentInstantAnalysis.Get(tabletAliasString); found {
 		if lastWrittenAnalysis == analysisCode {
 			// Surely nothing new.
 			// And let's expand the timeout
-			recentInstantAnalysis.Set(tabletAlias, analysisCode, cache.DefaultExpiration)
+			recentInstantAnalysis.Set(tabletAliasString, analysisCode, cache.DefaultExpiration)
 			return nil
 		}
 	}
@@ -590,7 +591,9 @@ func auditInstanceAnalysisInChangelog(tabletAlias string, analysisCode AnalysisC
 				alias = ?
 				AND analysis != ?
 			`,
-			string(analysisCode), tabletAlias, string(analysisCode),
+			string(analysisCode),
+			tabletAliasString,
+			string(analysisCode),
 		)
 		if err != nil {
 			log.Error(err.Error())
@@ -619,7 +622,8 @@ func auditInstanceAnalysisInChangelog(tabletAlias string, analysisCode AnalysisC
 				DATETIME('now'),
 				?
 			)`,
-			tabletAlias, string(analysisCode),
+			tabletAliasString,
+			string(analysisCode),
 		)
 		if err != nil {
 			log.Error(err.Error())
@@ -632,7 +636,7 @@ func auditInstanceAnalysisInChangelog(tabletAlias string, analysisCode AnalysisC
 		}
 		firstInsertion = rows > 0
 	}
-	recentInstantAnalysis.Set(tabletAlias, analysisCode, cache.DefaultExpiration)
+	recentInstantAnalysis.Set(tabletAliasString, analysisCode, cache.DefaultExpiration)
 	// If the analysis has changed or if it is the first insertion, we need to make sure we write this change to the database.
 	if !lastAnalysisChanged && !firstInsertion {
 		return nil
@@ -648,7 +652,8 @@ func auditInstanceAnalysisInChangelog(tabletAlias string, analysisCode AnalysisC
 			DATETIME('now'),
 			?
 		)`,
-		tabletAlias, string(analysisCode),
+		tabletAliasString,
+		string(analysisCode),
 	)
 	if err == nil {
 		analysisChangeWriteCounter.Add(1)