Fix bug in SwitchTraffic that wasn't respecting --dry_run for rea…

…donly and replica tablets during a resharding event (#12992) * use switcher struct when switching shard reads during a reshard event Signed-off-by: austenLacy <[email protected]> * Create failing test for bug reported in #12992, where a TrafficSwitch dry run for reads during resharding tries to actually switch reads and fails Signed-off-by: Rohit Nayak <[email protected]> --------- Signed-off-by: austenLacy <[email protected]> Signed-off-by: Rohit Nayak <[email protected]> Co-authored-by: Rohit Nayak <[email protected]>
vitessio · Apr 28, 2023 · dc5ec10 · dc5ec10
1 parent 9c80a07
commit dc5ec10
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 10 deletions.
diff --git a/go/test/endtoend/vreplication/vreplication_test.go b/go/test/endtoend/vreplication/vreplication_test.go
@@ -875,7 +875,7 @@ func reshardCustomer2to4Split(t *testing.T, cells []*Cell, sourceCellOrAlias str
 		ksName := "customer"
 		counts := map[string]int{"zone1-600": 4, "zone1-700": 5, "zone1-800": 6, "zone1-900": 5}
 		reshard(t, ksName, "customer", "c2c4", "-80,80-", "-40,40-80,80-c0,c0-",
-			600, counts, nil, cells, sourceCellOrAlias, 1)
+			600, counts, nil, nil, cells, sourceCellOrAlias, 1)
 		waitForRowCount(t, vtgateConn, ksName, "customer", 20)
 		query := "insert into customer (name) values('yoko')"
 		execVtgateQuery(t, vtgateConn, ksName, query)
@@ -888,7 +888,7 @@ func reshardMerchant2to3SplitMerge(t *testing.T) {
 		ksName := merchantKeyspace
 		counts := map[string]int{"zone1-1600": 0, "zone1-1700": 2, "zone1-1800": 0}
 		reshard(t, ksName, "merchant", "m2m3", "-80,80-", "-40,40-c0,c0-",
-			1600, counts, dryRunResultsSwitchWritesM2m3, nil, "", 1)
+			1600, counts, dryRunResultsSwitchReadM2m3, dryRunResultsSwitchWritesM2m3, nil, "", 1)
 		waitForRowCount(t, vtgateConn, ksName, "merchant", 2)
 		query := "insert into merchant (mname, category) values('amazon', 'electronics')"
 		execVtgateQuery(t, vtgateConn, ksName, query)
@@ -935,7 +935,7 @@ func reshardMerchant3to1Merge(t *testing.T) {
 		ksName := merchantKeyspace
 		counts := map[string]int{"zone1-2000": 3}
 		reshard(t, ksName, "merchant", "m3m1", "-40,40-c0,c0-", "0",
-			2000, counts, nil, nil, "", 1)
+			2000, counts, nil, nil, nil, "", 1)
 		waitForRowCount(t, vtgateConn, ksName, "merchant", 3)
 		query := "insert into merchant (mname, category) values('flipkart', 'electronics')"
 		execVtgateQuery(t, vtgateConn, ksName, query)
@@ -948,7 +948,7 @@ func reshardCustomer3to2SplitMerge(t *testing.T) { // -40,40-80,80-c0 => merge/s
 		ksName := "customer"
 		counts := map[string]int{"zone1-1000": 8, "zone1-1100": 8, "zone1-1200": 5}
 		reshard(t, ksName, "customer", "c4c3", "-40,40-80,80-c0", "-60,60-c0",
-			1000, counts, nil, nil, "", 1)
+			1000, counts, nil, nil, nil, "", 1)
 	})
 }
 
@@ -957,12 +957,12 @@ func reshardCustomer3to1Merge(t *testing.T) { // to unsharded
 		ksName := "customer"
 		counts := map[string]int{"zone1-1500": 21}
 		reshard(t, ksName, "customer", "c3c1", "-60,60-c0,c0-", "0",
-			1500, counts, nil, nil, "", 3)
+			1500, counts, nil, nil, nil, "", 3)
 	})
 }
 
 func reshard(t *testing.T, ksName string, tableName string, workflow string, sourceShards string, targetShards string,
-	tabletIDBase int, counts map[string]int, dryRunResultSwitchWrites []string, cells []*Cell, sourceCellOrAlias string,
+	tabletIDBase int, counts map[string]int, dryRunResultSwitchReads, dryRunResultSwitchWrites []string, cells []*Cell, sourceCellOrAlias string,
 	autoIncrementStep int) {
 	t.Run("reshard", func(t *testing.T) {
 		if cells == nil {
@@ -1004,6 +1004,9 @@ func reshard(t *testing.T, ksName string, tableName string, workflow string, sou
 			}
 		}
 		vdiff1(t, ksWorkflow, "")
+		if dryRunResultSwitchReads != nil {
+			switchReadsDryRun(t, workflowType, allCellNames, ksWorkflow, dryRunResultSwitchReads)
+		}
 		switchReads(t, workflowType, allCellNames, ksWorkflow, false)
 		if dryRunResultSwitchWrites != nil {
 			switchWritesDryRun(t, workflowType, ksWorkflow, dryRunResultSwitchWrites)
@@ -1361,7 +1364,9 @@ func switchReadsDryRun(t *testing.T, workflowType, cells, ksWorkflow string, dry
 	output, err := vc.VtctlClient.ExecuteCommandWithOutput(workflowType, "--", "--cells="+cells, "--tablet_types=rdonly,replica",
 		"--dry_run", "SwitchTraffic", ksWorkflow)
 	require.NoError(t, err, fmt.Sprintf("Switching Reads DryRun Error: %s: %s", err, output))
-	validateDryRunResults(t, output, dryRunResults)
+	if dryRunResults != nil {
+		validateDryRunResults(t, output, dryRunResults)
+	}
 }
 
 func switchReads(t *testing.T, workflowType, cells, ksWorkflow string, reverse bool) {

diff --git a/go/test/endtoend/vreplication/vreplication_test_env.go b/go/test/endtoend/vreplication/vreplication_test_env.go
@@ -85,3 +85,9 @@ var dryRunResultsSwitchWritesM2m3 = []string{
 	"       Keyspace merchant-type, Shard c0-, Tablet 1800, Workflow m2m3, DbName vt_merchant-type",
 	"Unlock keyspace merchant-type",
 }
+
+var dryRunResultsSwitchReadM2m3 = []string{
+	"Lock keyspace merchant-type",
+	"Switch reads from keyspace merchant-type to keyspace merchant-type for shards -80,80- to shards -40,40-c0,c0-",
+	"Unlock keyspace merchant-type",
+}
diff --git a/go/test/endtoend/vreplication/vstream_test.go b/go/test/endtoend/vreplication/vstream_test.go
@@ -372,7 +372,7 @@ func testVStreamStopOnReshardFlag(t *testing.T, stopOnReshard bool, baseTabletID
 		switch tickCount {
 		case 1:
 			reshard(t, "sharded", "customer", "vstreamStopOnReshard", "-80,80-",
-				"-40,40-", baseTabletID+400, nil, nil, nil, defaultCellName, 1)
+				"-40,40-", baseTabletID+400, nil, nil, nil, nil, defaultCellName, 1)
 		case 60:
 			done = true
 		}
@@ -502,7 +502,7 @@ func testVStreamCopyMultiKeyspaceReshard(t *testing.T, baseTabletID int) numEven
 		tickCount++
 		switch tickCount {
 		case 1:
-			reshard(t, "sharded", "customer", "vstreamCopyMultiKeyspaceReshard", "-80,80-", "-40,40-", baseTabletID+400, nil, nil, nil, defaultCellName, 1)
+			reshard(t, "sharded", "customer", "vstreamCopyMultiKeyspaceReshard", "-80,80-", "-40,40-", baseTabletID+400, nil, nil, nil, nil, defaultCellName, 1)
 			reshardDone = true
 		case 60:
 			done = true

diff --git a/go/vt/wrangler/traffic_switcher.go b/go/vt/wrangler/traffic_switcher.go
@@ -397,7 +397,7 @@ func (wr *Wrangler) SwitchReads(ctx context.Context, targetKeyspace, workflowNam
 		return sw.logs(), nil
 	}
 	wr.Logger().Infof("About to switchShardReads: %+v, %+v, %+v", cells, servedTypes, direction)
-	if err := ts.switchShardReads(ctx, cells, servedTypes, direction); err != nil {
+	if err := sw.switchShardReads(ctx, cells, servedTypes, direction); err != nil {
 		ts.Logger().Errorf("switchShardReads failed: %v", err)
 		return nil, err
 	}