From a847429605452e40c3430ad8e85a07213e360766 Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Tue, 10 Sep 2024 09:54:27 +0200 Subject: [PATCH] Address review comments Signed-off-by: Rohit Nayak --- go/vt/vtctl/workflow/framework_test.go | 8 +++++++- go/vt/vtctl/workflow/server.go | 5 +++-- go/vt/vtctl/workflow/workflow_state_test.go | 8 +++----- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/go/vt/vtctl/workflow/framework_test.go b/go/vt/vtctl/workflow/framework_test.go index 59653d7b906..9dd91d9d4b7 100644 --- a/go/vt/vtctl/workflow/framework_test.go +++ b/go/vt/vtctl/workflow/framework_test.go @@ -58,6 +58,12 @@ const ( tabletUIDStep = 10 ) +var defaultTabletTypes = []topodatapb.TabletType{ + topodatapb.TabletType_PRIMARY, + topodatapb.TabletType_REPLICA, + topodatapb.TabletType_RDONLY, +} + type testKeyspace struct { KeyspaceName string ShardNames []string @@ -209,7 +215,7 @@ func (env *testEnv) updateTableRoutingRules(t *testing.T, ctx context.Context, tabletTypes []topodatapb.TabletType, tables []string, sourceKeyspace, targetKeyspace, toKeyspace string) { if len(tabletTypes) == 0 { - tabletTypes = []topodatapb.TabletType{topodatapb.TabletType_PRIMARY, topodatapb.TabletType_REPLICA, topodatapb.TabletType_RDONLY} + tabletTypes = defaultTabletTypes } rr, err := env.ts.GetRoutingRules(ctx) require.NoError(t, err) diff --git a/go/vt/vtctl/workflow/server.go b/go/vt/vtctl/workflow/server.go index 10b92c78456..6a48cf90c2e 100644 --- a/go/vt/vtctl/workflow/server.go +++ b/go/vt/vtctl/workflow/server.go @@ -1034,8 +1034,9 @@ func (s *Server) getWorkflowState(ctx context.Context, targetKeyspace, workflowN for _, table := range ts.Tables() { // If a rule for the primary tablet type exists for any table and points to the target keyspace, // then writes have been switched. - rr := globalRules[fmt.Sprintf("%s.%s", sourceKeyspace, table)] - if len(rr) > 0 && rr[0] != fmt.Sprintf("%s.%s", sourceKeyspace, table) { + ruleKey := fmt.Sprintf("%s.%s", sourceKeyspace, table) + rr := globalRules[ruleKey] + if len(rr) > 0 && rr[0] != ruleKey { state.WritesSwitched = true break } diff --git a/go/vt/vtctl/workflow/workflow_state_test.go b/go/vt/vtctl/workflow/workflow_state_test.go index 188965a3fc3..d64b6b36a86 100644 --- a/go/vt/vtctl/workflow/workflow_state_test.go +++ b/go/vt/vtctl/workflow/workflow_state_test.go @@ -119,7 +119,7 @@ func TestWorkflowStateMoveTables(t *testing.T) { }, { name: "switch reads and writes", - wf1SwitchedTabletTypes: []topodatapb.TabletType{topodatapb.TabletType_PRIMARY, topodatapb.TabletType_REPLICA, topodatapb.TabletType_RDONLY}, + wf1SwitchedTabletTypes: defaultTabletTypes, wf1ExpectedState: "All Reads Switched. Writes Switched", }, { @@ -132,7 +132,7 @@ func TestWorkflowStateMoveTables(t *testing.T) { name: "switch replica only", wf1SwitchedTabletTypes: []topodatapb.TabletType{topodatapb.TabletType_REPLICA}, wf1ExpectedState: "Reads partially switched. All Replica Reads Switched. Rdonly not switched. Writes Not Switched", - wf2SwitchedTabletTypes: []topodatapb.TabletType{topodatapb.TabletType_PRIMARY, topodatapb.TabletType_REPLICA, topodatapb.TabletType_RDONLY}, + wf2SwitchedTabletTypes: defaultTabletTypes, }, } tables := []string{"t1"} @@ -152,16 +152,14 @@ func TestWorkflowStateMoveTables(t *testing.T) { te.updateTableRoutingRules(t, ctx, nil, tables, "source2", "target2", "source2") } - resetRoutingRules() for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + resetRoutingRules() te.updateTableRoutingRules(t, ctx, tc.wf1SwitchedTabletTypes, tables, "source", te.targetKeyspace.KeyspaceName, te.targetKeyspace.KeyspaceName) te.updateTableRoutingRules(t, ctx, tc.wf2SwitchedTabletTypes, tables, "source2", "target2", "target2") require.Equal(t, tc.wf1ExpectedState, getStateString("target", "wf1")) - // reset to initial state - resetRoutingRules() }) } }