From d2d5efe328cf72e4bfea12c9280269b3e1ed3d2f Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Wed, 22 May 2024 08:30:18 -0400 Subject: [PATCH] Don't fail workflow cancel/cleanup if there's nothing to do Signed-off-by: Matt Lord --- go/vt/topo/shard.go | 3 ++- go/vt/vtctl/workflow/server.go | 6 +++--- go/vt/vtctl/workflow/traffic_switcher.go | 23 +++++++++++++++++++---- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/go/vt/topo/shard.go b/go/vt/topo/shard.go index b9554bf789f..827b531e6a5 100644 --- a/go/vt/topo/shard.go +++ b/go/vt/topo/shard.go @@ -451,7 +451,8 @@ func (si *ShardInfo) updatePrimaryTabletControl(tc *topodatapb.Shard_TabletContr } if remove { if len(newTables) != 0 { - return vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, dlTablesNotPresent) + // These tables did not exist in the denied list so we don't need to remove them. + log.Warningf("%s:%s", dlTablesNotPresent, strings.Join(newTables, ",")) } var newDenyList []string if len(tables) != 0 { // legacy uses diff --git a/go/vt/vtctl/workflow/server.go b/go/vt/vtctl/workflow/server.go index 00c36b2d3dc..3ad8e72f5dd 100644 --- a/go/vt/vtctl/workflow/server.go +++ b/go/vt/vtctl/workflow/server.go @@ -1971,7 +1971,7 @@ func (s *Server) WorkflowDelete(ctx context.Context, req *vtctldatapb.WorkflowDe ts, state, err := s.getWorkflowState(ctx, req.GetKeyspace(), req.GetWorkflow()) if err != nil { - log.Errorf("Failed to get VReplication workflow state for %s.%s: %v", req.GetKeyspace(), req.GetWorkflow(), err) + log.Errorf("failed to get VReplication workflow state for %s.%s: %v", req.GetKeyspace(), req.GetWorkflow(), err) return nil, err } @@ -2807,8 +2807,8 @@ func (s *Server) DeleteShard(ctx context.Context, keyspace, shard string, recurs shardInfo, err := s.ts.GetShard(ctx, keyspace, shard) if err != nil { if topo.IsErrType(err, topo.NoNode) { - log.Infof("Shard %v/%v doesn't seem to exist, cleaning up any potential leftover", keyspace, shard) - return s.ts.DeleteShard(ctx, keyspace, shard) + log.Warningf("Shard %v/%v did not exist when attempting to remove it", keyspace, shard) + return nil } return err } diff --git a/go/vt/vtctl/workflow/traffic_switcher.go b/go/vt/vtctl/workflow/traffic_switcher.go index 9ea1c8b609b..1ac9441c56a 100644 --- a/go/vt/vtctl/workflow/traffic_switcher.go +++ b/go/vt/vtctl/workflow/traffic_switcher.go @@ -26,12 +26,11 @@ import ( "sync" "time" - vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata" - "golang.org/x/exp/maps" "golang.org/x/sync/errgroup" "vitess.io/vitess/go/json2" + "vitess.io/vitess/go/mysql/sqlerror" "vitess.io/vitess/go/sqlescape" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/binlog/binlogplayer" @@ -53,6 +52,7 @@ import ( tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" topodatapb "vitess.io/vitess/go/vt/proto/topodata" vschemapb "vitess.io/vitess/go/vt/proto/vschema" + vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) @@ -428,6 +428,10 @@ func (ts *trafficSwitcher) deleteShardRoutingRules(ctx context.Context) error { } srr, err := topotools.GetShardRoutingRules(ctx, ts.TopoServer()) if err != nil { + if topo.IsErrType(err, topo.NoNode) { + log.Warningf("No shard routing rules found when attempting to delete the ones for the %s keyspace", ts.targetKeyspace) + return nil + } return err } for _, si := range ts.TargetShards() { @@ -537,6 +541,10 @@ func (ts *trafficSwitcher) removeSourceTables(ctx context.Context, removalType T DisableForeignKeyChecks: true, }) if err != nil { + if mysqlErr, ok := err.(*sqlerror.SQLError); ok && mysqlErr.Num == sqlerror.ERNoSuchTable { + ts.Logger().Warningf("%s: Table %s did not exist when attempting to remove it", source.GetPrimary().String(), tableName) + return nil + } ts.Logger().Errorf("%s: Error removing table %s: %v", source.GetPrimary().String(), tableName, err) return err } @@ -833,6 +841,7 @@ func (ts *trafficSwitcher) deleteReverseVReplication(ctx context.Context) error return ts.ForAllSources(func(source *MigrationSource) error { query := fmt.Sprintf(sqlDeleteWorkflow, encodeString(source.GetPrimary().DbName()), encodeString(ts.reverseWorkflow)) if _, err := ts.TabletManagerClient().VReplicationExec(ctx, source.GetPrimary().Tablet, query); err != nil { + // vreplication.exec returns no error on delete if the rows do not exist. return err } ts.ws.deleteWorkflowVDiffData(ctx, source.GetPrimary().Tablet, ts.reverseWorkflow) @@ -1112,6 +1121,7 @@ func (ts *trafficSwitcher) dropTargetVReplicationStreams(ctx context.Context) er ts.Logger().Infof("Deleting target streams and related data for workflow %s db_name %s", ts.WorkflowName(), target.GetPrimary().DbName()) query := fmt.Sprintf(sqlDeleteWorkflow, encodeString(target.GetPrimary().DbName()), encodeString(ts.WorkflowName())) if _, err := ts.TabletManagerClient().VReplicationExec(ctx, target.GetPrimary().Tablet, query); err != nil { + // vreplication.exec returns no error on delete if the rows do not exist. return err } ts.ws.deleteWorkflowVDiffData(ctx, target.GetPrimary().Tablet, ts.WorkflowName()) @@ -1125,6 +1135,7 @@ func (ts *trafficSwitcher) dropSourceReverseVReplicationStreams(ctx context.Cont ts.Logger().Infof("Deleting reverse streams and related data for workflow %s db_name %s", ts.WorkflowName(), source.GetPrimary().DbName()) query := fmt.Sprintf(sqlDeleteWorkflow, encodeString(source.GetPrimary().DbName()), encodeString(ReverseWorkflowName(ts.WorkflowName()))) if _, err := ts.TabletManagerClient().VReplicationExec(ctx, source.GetPrimary().Tablet, query); err != nil { + return err } ts.ws.deleteWorkflowVDiffData(ctx, source.GetPrimary().Tablet, ReverseWorkflowName(ts.WorkflowName())) @@ -1156,8 +1167,12 @@ func (ts *trafficSwitcher) removeTargetTables(ctx context.Context) error { }) log.Infof("Removed target table with result: %+v", res) if err != nil { - ts.Logger().Errorf("%s: Error removing table %s: %v", - target.GetPrimary().String(), tableName, err) + if mysqlErr, ok := err.(*sqlerror.SQLError); ok && mysqlErr.Num == sqlerror.ERNoSuchTable { + // The table was already gone, so we can ignore the error. + ts.Logger().Warningf("%s: Table %s did not exist when attempting to remove it", target.GetPrimary().String(), tableName) + return nil + } + ts.Logger().Errorf("%s: Error removing table %s: %v", target.GetPrimary().String(), tableName, err) return err } ts.Logger().Infof("%s: Removed table %s.%s\n",