Minor tweaks after self review

Signed-off-by: Matt Lord <[email protected]>
vitessio · Sep 28, 2023 · 8f25f73 · 8f25f73
1 parent ac7729d
commit 8f25f73
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 38 deletions.
diff --git a/go/vt/vtctl/workflow/materializer_env_test.go b/go/vt/vtctl/workflow/materializer_env_test.go
@@ -306,13 +306,3 @@ func (tmc *testMaterializerTMClient) ApplySchema(ctx context.Context, tablet *to
 
 	return nil, nil
 }
-
-func (tmc *testMaterializerTMClient) VDiff(ctx context.Context, tablet *topodatapb.Tablet, req *tabletmanagerdatapb.VDiffRequest) (*tabletmanagerdatapb.VDiffResponse, error) {
-	return &tabletmanagerdatapb.VDiffResponse{
-		Id:        1,
-		VdiffUuid: req.VdiffUuid,
-		Output: &querypb.QueryResult{
-			RowsAffected: 1,
-		},
-	}, nil
-}
diff --git a/go/vt/vtctl/workflow/server.go b/go/vt/vtctl/workflow/server.go
@@ -1990,7 +1990,7 @@ func (s *Server) deleteWorkflowVDiffData(ctx context.Context, tablet *topodatapb
 		Keyspace:  tablet.Keyspace,
 		Workflow:  workflow,
 		Action:    string(vdiff.DeleteAction),
-		ActionArg: "all",
+		ActionArg: vdiff.AllActionArg,
 	}); err != nil {
 		log.Errorf("Error deleting vdiff data for %s.%s workflow: %v", tablet.Keyspace, workflow, err)
 	}

diff --git a/go/vt/vttablet/tabletmanager/vdiff/action.go b/go/vt/vttablet/tabletmanager/vdiff/action.go
@@ -354,14 +354,20 @@ func (vde *Engine) handleStopAction(ctx context.Context, dbClient binlogplayer.D
 func (vde *Engine) handleDeleteAction(ctx context.Context, dbClient binlogplayer.DBClient, action VDiffAction, req *tabletmanagerdatapb.VDiffRequest, resp *tabletmanagerdatapb.VDiffResponse) error {
 	vde.mu.Lock()
 	defer vde.mu.Unlock()
-	var err error
-	var query string
+	var deleteQuery string
+	cleanupController := func(controller *controller) {
+		if controller == nil {
+			return
+		}
+		controller.Stop()
+		delete(vde.controllers, controller.id)
+	}
 
 	switch req.ActionArg {
 	case AllActionArg:
 		// We need to stop any running controllers before we delete
 		// the vdiff records.
-		query, err = sqlparser.ParseAndBind(sqlGetVDiffIDsByKeyspaceWorkflow,
+		query, err := sqlparser.ParseAndBind(sqlGetVDiffIDsByKeyspaceWorkflow,
 			sqltypes.StringBindVariable(req.Keyspace),
 			sqltypes.StringBindVariable(req.Workflow),
 		)
@@ -373,12 +379,9 @@ func (vde *Engine) handleDeleteAction(ctx context.Context, dbClient binlogplayer
 			return err
 		}
 		for _, row := range res.Named().Rows {
-			if controller := vde.controllers[row.AsInt64("id", -1)]; controller != nil {
-				controller.Stop()
-				delete(vde.controllers, controller.id)
-			}
+			cleanupController(vde.controllers[row.AsInt64("id", -1)])
 		}
-		query, err = sqlparser.ParseAndBind(sqlDeleteVDiffs,
+		deleteQuery, err = sqlparser.ParseAndBind(sqlDeleteVDiffs,
 			sqltypes.StringBindVariable(req.Keyspace),
 			sqltypes.StringBindVariable(req.Workflow),
 		)
@@ -392,7 +395,7 @@ func (vde *Engine) handleDeleteAction(ctx context.Context, dbClient binlogplayer
 		}
 		// We need to be sure that the controller is stopped, if
 		// it's still running, before we delete the vdiff record.
-		query, err = sqlparser.ParseAndBind(sqlGetVDiffID,
+		query, err := sqlparser.ParseAndBind(sqlGetVDiffID,
 			sqltypes.StringBindVariable(uuid.String()),
 		)
 		if err != nil {
@@ -407,20 +410,16 @@ func (vde *Engine) handleDeleteAction(ctx context.Context, dbClient binlogplayer
 			return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "no vdiff found for UUID %s on tablet %v",
 				uuid, vde.thisTablet.Alias)
 		}
-		controller, ok := vde.controllers[row.AsInt64("id", -1)]
-		if ok {
-			controller.Stop()
-			delete(vde.controllers, controller.id)
-		}
-		query, err = sqlparser.ParseAndBind(sqlDeleteVDiffByUUID,
+		cleanupController(vde.controllers[row.AsInt64("id", -1)])
+		deleteQuery, err = sqlparser.ParseAndBind(sqlDeleteVDiffByUUID,
 			sqltypes.StringBindVariable(uuid.String()),
 		)
 		if err != nil {
 			return err
 		}
 	}
 	// Execute the query which deletes the vdiff record(s).
-	if _, err = dbClient.ExecuteFetch(query, 1); err != nil {
+	if _, err := dbClient.ExecuteFetch(deleteQuery, 1); err != nil {
 		return err
 	}
 

diff --git a/go/vt/wrangler/workflow.go b/go/vt/wrangler/workflow.go
@@ -718,7 +718,7 @@ func (wr *Wrangler) deleteWorkflowVDiffData(ctx context.Context, tablet *topodat
 		Keyspace:  tablet.Keyspace,
 		Workflow:  workflow,
 		Action:    string(vdiff2.DeleteAction),
-		ActionArg: "all",
+		ActionArg: vdiff2.AllActionArg,
 	}); err != nil {
 		log.Errorf("Error deleting vdiff data for %s.%s workflow: %v", tablet.Keyspace, workflow, err)
 	}

diff --git a/go/vt/wrangler/wrangler_env_test.go b/go/vt/wrangler/wrangler_env_test.go
@@ -341,13 +341,3 @@ func (tmc *testWranglerTMClient) ExecuteFetchAsApp(ctx context.Context, tablet *
 	}
 	return result, nil
 }
-
-func (tmc *testWranglerTMClient) VDiff(ctx context.Context, tablet *topodatapb.Tablet, req *tabletmanagerdatapb.VDiffRequest) (*tabletmanagerdatapb.VDiffResponse, error) {
-	return &tabletmanagerdatapb.VDiffResponse{
-		Id:        1,
-		VdiffUuid: req.VdiffUuid,
-		Output: &querypb.QueryResult{
-			RowsAffected: 1,
-		},
-	}, nil
-}