From 8f25f7394c13b1f18b9d21f70fdc7afb554daa42 Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Thu, 28 Sep 2023 00:13:21 -0400 Subject: [PATCH] Minor tweaks after self review Signed-off-by: Matt Lord --- go/vt/vtctl/workflow/materializer_env_test.go | 10 ------ go/vt/vtctl/workflow/server.go | 2 +- go/vt/vttablet/tabletmanager/vdiff/action.go | 31 +++++++++---------- go/vt/wrangler/workflow.go | 2 +- go/vt/wrangler/wrangler_env_test.go | 10 ------ 5 files changed, 17 insertions(+), 38 deletions(-) diff --git a/go/vt/vtctl/workflow/materializer_env_test.go b/go/vt/vtctl/workflow/materializer_env_test.go index f02a6079b48..73646d7da80 100644 --- 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 index 27b020d0c62..c4d91ae9c6d 100644 --- 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 index 7c4d2032d7f..ac9bec86990 100644 --- 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,12 +410,8 @@ 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 { @@ -420,7 +419,7 @@ func (vde *Engine) handleDeleteAction(ctx context.Context, dbClient binlogplayer } } // 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 index 055c14a0da5..d9dbcee7291 100644 --- 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 index d952705e982..4dd5e342c35 100644 --- 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 -}