VReplication: disable foreign_key_checks for bulk data cleanup

[mattlord]

Description

VReplication performs bulk data loads and cleanup (source side on complete and target side on cancel) and for the load portion it disabled foreign_key_checks (as do other bulk data load tools such as mysqldump), however, it did NOT do this for the bulk data cleanup. This PR also disables foreign_key_checks for the cleanup step to ensure that we can successfully cleanup the data when the tables being dropped/renamed (rename only an option on complete for the original source tables) have dependent constraints.

This doesn't necessarily have to be backported to v19, but I thought we should because foreign key support exists there and this fixes an important bug related to that support AND v19 is still in RC.

Related Issue(s)

• Fixes: Bug Report: Unable to cancel movetables with keep_data=false on database with foreign keys #15232

Checklist

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[rohit-nayak-ps]

This should be changed to workflowFlavorRandom and also tested locally with workflowFlavorVtctld

[mattlord]

Tested locally with workflowFlavorVtctld and it passes:

❯ git diff | cat
diff --git a/go/test/endtoend/vreplication/fk_test.go b/go/test/endtoend/vreplication/fk_test.go
index b7a88aee65..b15856501a 100644
--- a/go/test/endtoend/vreplication/fk_test.go
+++ b/go/test/endtoend/vreplication/fk_test.go
@@ -34,7 +34,7 @@ import (
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 )

-const testWorkflowFlavor = workflowFlavorVtctl
+const testWorkflowFlavor = workflowFlavorVtctld

 // TestFKWorkflow runs a MoveTables workflow with atomic copy for a db with foreign key constraints.
 // It inserts initial data, then simulates load. We insert both child rows with foreign keys and those without,

 ❯ make build
Thu Feb 15 15:12:32 EST 2024: Building source tree

❯ go test -v -timeout 15m -run ^TestFKWorkflow$ vitess.io/vitess/go/test/endtoend/vreplication
VTDATAROOT is /opt/vtdataroot/vreple2e_687716
=== RUN   TestFKWorkflow
E0215 15:13:19.528356   41743 vtorc_process.go:103] configuration - {
	"Debug": true,
	"ListenAddress": ":0",
	"MySQLTopologyUser": "orc_client_user",
	"MySQLTopologyPassword": "orc_client_user_password",
	"MySQLReplicaUser": "vt_repl",
	"MySQLReplicaPassword": "",
	"RecoveryPeriodBlockSeconds": 1
}
=== RUN   TestFKWorkflow/insertInitialFKData
=== RUN   TestFKWorkflow/vtctlclient_vdiff_fktarget.fk
=== RUN   TestFKWorkflow/vtctlclient_vdiff_fktarget.fk#01
--- PASS: TestFKWorkflow (52.09s)
    --- PASS: TestFKWorkflow/insertInitialFKData (0.01s)
    --- PASS: TestFKWorkflow/vtctlclient_vdiff_fktarget.fk (5.06s)
    --- PASS: TestFKWorkflow/vtctlclient_vdiff_fktarget.fk#01 (5.06s)
PASS
ok  	vitess.io/vitess/go/test/endtoend/vreplication	53.235s

Will change it to random now and push that change.

[rohit-nayak-ps]

Nice compact solution.

[codecov]

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (de3c9c5) 67.46% compared to head (c95720e) 67.47%.

Files	Patch %	Lines
go/vt/vtctl/workflow/traffic_switcher.go	0.00%	8 Missing ⚠️
go/vt/vttablet/tabletmanager/rpc_query.go	75.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #15261   +/-   ##
=======================================
  Coverage   67.46%   67.47%           
=======================================
  Files        1560     1560           
  Lines      193186   193205   +19     
=======================================
+ Hits       130333   130359   +26     
+ Misses      62853    62846    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mattlord]

This addresses unrelated test flakes seen.