Add master tablet type in addition to replica as default source tablet types

Signed-off-by: Rohit Nayak <rohit@planetscale.com>

go.mod

@@ -62,6 +62,7 @@ require (
 	github.com/martini-contrib/render v0.0.0-20150707142108-ec18f8345a11
 	github.com/mattn/go-sqlite3 v1.14.0
 	github.com/minio/minio-go v0.0.0-20190131015406-c8a261de75c1
+	github.com/mitchellh/go-ps v1.0.0 // indirect
 	github.com/mitchellh/go-testing-interface v1.14.0 // indirect
 	github.com/mitchellh/mapstructure v1.2.3 // indirect
 	github.com/montanaflynn/stats v0.6.3

go.sum

@@ -488,6 +488,8 @@ github.com/mitchellh/cli v1.1.0/go.mod h1:xcISNoH86gajksDmfB23e/pu+B+GeFRMYmoHXx
 github.com/mitchellh/go-homedir v1.0.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
+github.com/mitchellh/go-ps v1.0.0 h1:i6ampVEEF4wQFF+bkYfwYgY+F/uYJDktmvLPf7qIgjc=
+github.com/mitchellh/go-ps v1.0.0/go.mod h1:J4lOc8z8yJs6vUwklHw2XEIiT4z4C40KtWVN3nvg8Pg=
 github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eIhFbs2+BFgRtFPeD/KE+zxI=
 github.com/mitchellh/go-testing-interface v1.14.0 h1:/x0XQ6h+3U3nAyk1yx+bHPURrKa9sVVvYbuqZ7pIAtI=
 github.com/mitchellh/go-testing-interface v1.14.0/go.mod h1:gfgS7OtZj6MA4U1UrDRp04twqAjfvlZyCfX3sDjEym8=

go/test/endtoend/vreplication/resharding_workflows_v2_test.go

@@ -523,7 +523,7 @@ func moveCustomerTableSwitchFlows(t *testing.T, cells []*Cell, sourceCellOrAlias
 		switchWrites(t, ksWorkflow, false)
 		validateWritesRouteToTarget(t)
 
-		switchWrites(t, ksWorkflow, true)
+		switchWrites(t, reverseKsWorkflow, true)
 		validateWritesRouteToSource(t)
 
 		validateReadsRouteToSource(t, "replica")

go/test/endtoend/vreplication/vreplication_test.go

@@ -637,7 +637,9 @@ func switchReadsDryRun(t *testing.T, cells, ksWorkflow string, dryRunResults []s
 }
 
 func switchReads(t *testing.T, cells, ksWorkflow string) {
-	output, err := vc.VtctlClient.ExecuteCommandWithOutput("SwitchReads", "-cells="+cells, "-tablet_type=rdonly", ksWorkflow)
+	var output string
+	var err error
+	output, err = vc.VtctlClient.ExecuteCommandWithOutput("SwitchReads", "-cells="+cells, "-tablet_type=rdonly", ksWorkflow)
 	require.NoError(t, err, fmt.Sprintf("SwitchReads Error: %s: %s", err, output))
 	output, err = vc.VtctlClient.ExecuteCommandWithOutput("SwitchReads", "-cells="+cells, "-tablet_type=replica", ksWorkflow)
 	require.NoError(t, err, fmt.Sprintf("SwitchReads Error: %s: %s", err, output))

go/vt/vtctl/vtctl.go

@@ -2200,6 +2200,7 @@ func commandVRWorkflow(ctx context.Context, wr *wrangler.Wrangler, subFlags *fla
 			case progress := <-progressCh:
 				if progress.running == progress.total {
 					wr.Logger().Printf("\nWorkflow started successfully with %d stream(s)\n", progress.total)
+					printDetails()
 					return nil
 				}
 				wr.Logger().Printf("%d%% ... ", 100*progress.running/progress.total)

go/vt/vtgate/discoverygateway_test.go

@@ -171,7 +171,6 @@ func TestDiscoveryGatewayWaitForTablets(t *testing.T) {
 			},
 		},
 	}
-
 	dg := NewDiscoveryGateway(context.Background(), hc, srvTopo, "local", 2)
 
 	// replica should only use local ones

go/vt/wrangler/traffic_switcher.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 	"sync"
 	"time"
+	"vitess.io/vitess/go/vt/topotools"
 
 	"vitess.io/vitess/go/vt/vtgate/evalengine"
 
@@ -346,6 +347,39 @@ func (wr *Wrangler) getWorkflowState(ctx context.Context, targetKeyspace, workfl
 	return ts, ws, nil
 }
 
+func (wr *Wrangler) doCellsHaveRdonlyTablets(ctx context.Context, cells []string) (bool, error) {
+	areAnyRdonly := func(tablets []*topo.TabletInfo) bool {
+		for _, tablet := range tablets {
+			if tablet.Type == topodatapb.TabletType_RDONLY {
+				return true
+			}
+		}
+		return false
+	}
+
+	if len(cells) == 0 {
+		tablets, err := topotools.GetAllTabletsAcrossCells(ctx, wr.ts)
+		if err != nil {
+			return false, err
+		}
+		if areAnyRdonly(tablets) {
+			return true, nil
+		}
+
+	} else {
+		for _, cell := range cells {
+			tablets, err := topotools.GetAllTablets(ctx, wr.ts, cell)
+			if err != nil {
+				return false, err
+			}
+			if areAnyRdonly(tablets) {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
 // SwitchReads is a generic way of switching read traffic for a resharding workflow.
 func (wr *Wrangler) SwitchReads(ctx context.Context, targetKeyspace, workflow string, servedTypes []topodatapb.TabletType,
 	cells []string, direction TrafficSwitchDirection, dryRun bool) (*[]string, error) {
@@ -360,7 +394,8 @@ func (wr *Wrangler) SwitchReads(ctx context.Context, targetKeyspace, workflow st
 		wr.Logger().Errorf(errorMsg)
 		return nil, fmt.Errorf(errorMsg)
 	}
-	wr.Logger().Infof("SwitchReads: %s.%s tt %+v, cells %+v, workflow state: %+v", targetKeyspace, workflow, servedTypes, cells, ws)
+	log.Infof("SwitchReads: %s.%s tt %+v, cells %+v, workflow state: %+v", targetKeyspace, workflow, servedTypes, cells, ws)
+	var switchReplicas, switchRdonly bool
 	for _, servedType := range servedTypes {
 		if servedType != topodatapb.TabletType_REPLICA && servedType != topodatapb.TabletType_RDONLY {
 			return nil, fmt.Errorf("tablet type must be REPLICA or RDONLY: %v", servedType)
@@ -371,6 +406,26 @@ func (wr *Wrangler) SwitchReads(ctx context.Context, targetKeyspace, workflow st
 		if direction == DirectionBackward && servedType == topodatapb.TabletType_RDONLY && len(ws.RdonlyCellsSwitched) == 0 {
 			return nil, fmt.Errorf("requesting reversal of SwitchReads for RDONLYs but RDONLY reads have not been switched")
 		}
+		switch servedType {
+		case topodatapb.TabletType_REPLICA:
+			switchReplicas = true
+		case topodatapb.TabletType_RDONLY:
+			switchRdonly = true
+		}
+	}
+
+	// if there are no rdonly tablets in the cells ask to switch rdonly tablets as well so that routing rules
+	// are updated for rdonly as well. Otherwise vitess will not know that the workflow has completed and will
+	// incorrectly report that not all reads have been switched. User currently is forced to switch non-existent rdonly tablets
+	if switchReplicas && !switchRdonly {
+		var err error
+		rdonlyTabletsExist, err := wr.doCellsHaveRdonlyTablets(ctx, cells)
+		if err != nil {
+			return nil, err
+		}
+		if !rdonlyTabletsExist {
+			servedTypes = append(servedTypes, topodatapb.TabletType_RDONLY)
+		}
 	}
 
 	// If journals exist notify user and fail
@@ -380,7 +435,7 @@ func (wr *Wrangler) SwitchReads(ctx context.Context, targetKeyspace, workflow st
 		return nil, err
 	}
 	if journalsExist {
-		wr.Logger().Errorf("Found a previous journal entry for %d", ts.id)
+		log.Infof("Found a previous journal entry for %d", ts.id)
 	}
 	var sw iswitcher
 	if dryRun {

go/vt/wrangler/traffic_switcher_test.go

@@ -177,12 +177,12 @@ func TestTableMigrateMainflow(t *testing.T) {
 		"ks2.t1":         {"ks1.t1"},
 		"t2":             {"ks1.t2"},
 		"ks2.t2":         {"ks1.t2"},
-		"t1@rdonly":      {"ks2.t1"},
-		"ks2.t1@rdonly":  {"ks2.t1"},
-		"ks1.t1@rdonly":  {"ks2.t1"},
-		"t2@rdonly":      {"ks2.t2"},
-		"ks2.t2@rdonly":  {"ks2.t2"},
-		"ks1.t2@rdonly":  {"ks2.t2"},
+		"t1@rdonly":      {"ks1.t1"},
+		"ks2.t1@rdonly":  {"ks1.t1"},
+		"ks1.t1@rdonly":  {"ks1.t1"},
+		"t2@rdonly":      {"ks1.t2"},
+		"ks2.t2@rdonly":  {"ks1.t2"},
+		"ks1.t2@rdonly":  {"ks1.t2"},
 		"t1@replica":     {"ks1.t1"},
 		"ks2.t1@replica": {"ks1.t1"},
 		"ks1.t1@replica": {"ks1.t1"},
@@ -526,10 +526,10 @@ func TestShardMigrateMainflow(t *testing.T) {
 	checkCellServedTypes(t, tme.ts, "ks:40-", "cell1", 2)
 	checkCellServedTypes(t, tme.ts, "ks:-80", "cell1", 1)
 	checkCellServedTypes(t, tme.ts, "ks:80-", "cell1", 1)
-	checkCellServedTypes(t, tme.ts, "ks:-40", "cell2", 2)
-	checkCellServedTypes(t, tme.ts, "ks:40-", "cell2", 2)
-	checkCellServedTypes(t, tme.ts, "ks:-80", "cell2", 1)
-	checkCellServedTypes(t, tme.ts, "ks:80-", "cell2", 1)
+	checkCellServedTypes(t, tme.ts, "ks:-40", "cell2", 1)
+	checkCellServedTypes(t, tme.ts, "ks:40-", "cell2", 1)
+	checkCellServedTypes(t, tme.ts, "ks:-80", "cell2", 2)
+	checkCellServedTypes(t, tme.ts, "ks:80-", "cell2", 2)
 	verifyQueries(t, tme.allDBClients)
 
 	tme.expectNoPreviousJournals()
@@ -1764,7 +1764,7 @@ func checkCellRouting(t *testing.T, wr *Wrangler, cell string, want map[string][
 		got[rr.FromTable] = append(got[rr.FromTable], rr.ToTables...)
 	}
 	if !reflect.DeepEqual(got, want) {
-		t.Errorf("srv rules for cell %s:\n%v, want\n%v", cell, got, want)
+		t.Fatalf("ERROR: routing rules don't match for cell %s:got\n%v, want\n%v", cell, got, want)
 	}
 }
 
@@ -1799,10 +1799,8 @@ func checkServedTypes(t *testing.T, ts *topo.Server, keyspaceShard string, want
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	if len(servedTypes) != want {
-		t.Errorf("shard %v has wrong served types: got: %v, want: %v", keyspaceShard, len(servedTypes), want)
-	}
+	require.Equal(t, want, len(servedTypes), fmt.Sprintf("shard %v has wrong served types: got: %v, want: %v",
+		keyspaceShard, len(servedTypes), want))
 }
 
 func checkCellServedTypes(t *testing.T, ts *topo.Server, keyspaceShard, cell string, want int) {
@@ -1823,9 +1821,8 @@ outer:
 			}
 		}
 	}
-	if count != want {
-		t.Errorf("serving types for keyspaceShard %s, cell %s: %d, want %d", keyspaceShard, cell, count, want)
-	}
+	require.Equal(t, want, count, fmt.Sprintf("serving types for keyspaceShard %s, cell %s: %d, want %d",
+		keyspaceShard, cell, count, want))
 }
 
 func checkIsMasterServing(t *testing.T, ts *topo.Server, keyspaceShard string, want bool) {

go/vt/wrangler/workflow_test.go

@@ -264,19 +264,19 @@ func TestMoveTablesV2Partial(t *testing.T) {
 	expectMoveTablesQueries(t, tme)
 
 	tme.expectNoPreviousJournals()
-	wf.params.TabletTypes = "replica"
+	wf.params.TabletTypes = "rdonly"
 	wf.params.Cells = "cell1"
 	require.NoError(t, testSwitchForward(t, wf))
 	require.Equal(t, "Reads partially switched. Replica switched in cells: cell1. Rdonly not switched. Writes Not Switched", wf.CurrentState())
 
 	tme.expectNoPreviousJournals()
-	wf.params.TabletTypes = "replica"
+	wf.params.TabletTypes = "rdonly"
 	wf.params.Cells = "cell2"
 	require.NoError(t, testSwitchForward(t, wf))
 	require.Equal(t, "Reads partially switched. All Replica Reads Switched. Rdonly not switched. Writes Not Switched", wf.CurrentState())
 
 	tme.expectNoPreviousJournals()
-	wf.params.TabletTypes = "rdonly"
+	wf.params.TabletTypes = "replica"
 	wf.params.Cells = "cell1,cell2"
 	require.NoError(t, testSwitchForward(t, wf))
 	require.Equal(t, WorkflowStateReadsSwitched, wf.CurrentState())
@@ -287,17 +287,16 @@ func TestMoveTablesV2Partial(t *testing.T) {
 	require.Equal(t, WorkflowStateNotSwitched, wf.CurrentState())
 
 	tme.expectNoPreviousJournals()
-	wf.params.TabletTypes = "rdonly"
+	wf.params.TabletTypes = "replica"
 	wf.params.Cells = "cell1"
 	require.NoError(t, testSwitchForward(t, wf))
 	require.Equal(t, "Reads partially switched. Replica not switched. Rdonly switched in cells: cell1. Writes Not Switched", wf.CurrentState())
 
 	tme.expectNoPreviousJournals()
-	wf.params.TabletTypes = "rdonly"
+	wf.params.TabletTypes = "replica"
 	wf.params.Cells = "cell2"
 	require.NoError(t, testSwitchForward(t, wf))
 	require.Equal(t, "Reads partially switched. Replica not switched. All Rdonly Reads Switched. Writes Not Switched", wf.CurrentState())
-
 }
 
 func TestMoveTablesV2Cancel(t *testing.T) {