Manual backport of vitessio#17300 (#570)

This PR is a manual backport of upstream vitessio#17300 The information below is a copy of what the description of the upstream PR This PR addresses fixes issues in PRS & ERS preference for tablets that are not taking backups. It does away with redundant field definitions in some of the proto messages and addresses segfault that could take place during ERS.
slackhq · Dec 3, 2024 · a03067d · a03067d
1 parent 2f80169
commit a03067d
Show file tree

Hide file tree

Showing 11 changed files with 385 additions and 590 deletions.
diff --git a/go/vt/proto/replicationdata/replicationdata.pb.go b/go/vt/proto/replicationdata/replicationdata.pb.go
diff --git a/go/vt/proto/replicationdata/replicationdata_vtproto.pb.go b/go/vt/proto/replicationdata/replicationdata_vtproto.pb.go
diff --git a/go/vt/proto/tabletmanagerdata/tabletmanagerdata.pb.go b/go/vt/proto/tabletmanagerdata/tabletmanagerdata.pb.go
diff --git a/go/vt/proto/tabletmanagerdata/tabletmanagerdata_vtproto.pb.go b/go/vt/proto/tabletmanagerdata/tabletmanagerdata_vtproto.pb.go
diff --git a/go/vt/vtctl/reparentutil/replication.go b/go/vt/vtctl/reparentutil/replication.go
@@ -250,9 +250,6 @@ func stopReplicationAndBuildStatusMaps(
 		logger.Infof("getting replication position from %v", alias)
 
 		stopReplicationStatus, err := tmc.StopReplicationAndGetStatus(groupCtx, tabletInfo.Tablet, replicationdatapb.StopReplicationMode_IOTHREADONLY)
-		m.Lock()
-		res.tabletsBackupState[alias] = stopReplicationStatus.GetBackupRunning()
-		m.Unlock()
 		if err != nil {
 			sqlErr, isSQLErr := mysql.NewSQLErrorFromError(err).(*mysql.SQLError)
 			if isSQLErr && sqlErr != nil && sqlErr.Number() == mysql.ERNotReplica {
@@ -276,6 +273,20 @@ func stopReplicationAndBuildStatusMaps(
 				err = vterrors.Wrapf(err, "error when getting replication status for alias %v: %v", alias, err)
 			}
 		} else {
+			isTakingBackup := false
+
+			// Prefer the most up-to-date information regarding whether the tablet is taking a backup from the After
+			// replication status, but fall back to the Before status if After is nil.
+			if stopReplicationStatus.After != nil {
+				isTakingBackup = stopReplicationStatus.After.BackupRunning
+			} else if stopReplicationStatus.Before != nil {
+				isTakingBackup = stopReplicationStatus.Before.BackupRunning
+			}
+
+			m.Lock()
+			res.tabletsBackupState[alias] = isTakingBackup
+			m.Unlock()
+
 			var sqlThreadRunning bool
 			// Check if the sql thread was running for the tablet
 			sqlThreadRunning, err = SQLThreadWasRunning(stopReplicationStatus)

diff --git a/go/vt/vttablet/grpctmserver/server.go b/go/vt/vttablet/grpctmserver/server.go
@@ -259,8 +259,6 @@ func (s *server) ReplicationStatus(ctx context.Context, request *tabletmanagerda
 		response.Status = status
 	}
 
-	response.BackupRunning = s.tm.IsBackupRunning()
-
 	return response, err
 }
 

diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go
@@ -61,7 +61,11 @@ func (tm *TabletManager) ReplicationStatus(ctx context.Context) (*replicationdat
 	if err != nil {
 		return nil, err
 	}
-	return mysql.ReplicationStatusToProto(status), nil
+
+	protoStatus := mysql.ReplicationStatusToProto(status)
+	protoStatus.BackupRunning = tm.IsBackupRunning()
+
+	return protoStatus, nil
 }
 
 // FullStatus returns the full status of MySQL including the replication information, semi-sync information, GTID information among others
@@ -909,6 +913,7 @@ func (tm *TabletManager) StopReplicationAndGetStatus(ctx context.Context, stopRe
 		return StopReplicationAndGetStatusResponse{}, vterrors.Wrap(err, "before status failed")
 	}
 	before := mysql.ReplicationStatusToProto(rs)
+	before.BackupRunning = tm.IsBackupRunning()
 
 	if stopReplicationMode == replicationdatapb.StopReplicationMode_IOTHREADONLY {
 		if !rs.IOHealthy() {
@@ -955,6 +960,7 @@ func (tm *TabletManager) StopReplicationAndGetStatus(ctx context.Context, stopRe
 		}, vterrors.Wrap(err, "acquiring replication status failed")
 	}
 	after := mysql.ReplicationStatusToProto(rsAfter)
+	after.BackupRunning = tm.IsBackupRunning()
 
 	rs.Position = rsAfter.Position
 	rs.RelayLogPosition = rsAfter.RelayLogPosition

diff --git a/proto/replicationdata.proto b/proto/replicationdata.proto
@@ -57,7 +57,6 @@ message Status {
 message StopReplicationStatus {
   replicationdata.Status before = 1;
   replicationdata.Status after = 2;
-  bool backup_running = 3;
 }
 
 // StopReplicationMode is used to provide controls over how replication is stopped.

diff --git a/proto/tabletmanagerdata.proto b/proto/tabletmanagerdata.proto
@@ -279,7 +279,6 @@ message ReplicationStatusRequest {
 
 message ReplicationStatusResponse {
   replicationdata.Status status = 1;
-  bool backup_running = 2;
 }
 
 message PrimaryStatusRequest {
@@ -460,7 +459,6 @@ message StopReplicationAndGetStatusResponse {
 
   // Status represents the replication status call right before, and right after telling the replica to stop.
   replicationdata.StopReplicationStatus status = 2;
-  bool backup_running = 3;
 }
 
 message PromoteReplicaRequest {

diff --git a/web/vtadmin/src/proto/vtadmin.d.ts b/web/vtadmin/src/proto/vtadmin.d.ts
-Original file line number
+Diff line change
@@ Expand Up @@
     		response.Status = status
     	}
-    	response.BackupRunning = s.tm.IsBackupRunning()
     	return response, err
     }
@@ Expand Down @@