VDiff: Improve row diff handling in report (#16618)

Signed-off-by: Matt Lord <[email protected]>

go/cmd/vtctldclient/command/vreplication/vdiff/vdiff.go

@@ -69,6 +69,7 @@ var (
 		WaitUpdateInterval          time.Duration
 		AutoRetry                   bool
 		MaxDiffDuration             time.Duration
+		RowDiffColumnTruncateAt     int64
 	}{}
 
 	deleteOptions = struct {
@@ -142,7 +143,7 @@ var (
 		Use:   "create",
 		Short: "Create and run a VDiff to compare the tables involved in a VReplication workflow between the source and target.",
 		Example: `vtctldclient --server localhost:15999 vdiff --workflow commerce2customer --target-keyspace customer create
-vtctldclient --server localhost:15999 vdiff --workflow commerce2customer --target-keyspace customer create b3f59678-5241-11ee-be56-0242ac120002`,
+vtctldclient --server :15999 vdiff --workflow c2c --target-keyspace customer create b3f59678-5241-11ee-be56-0242ac120002 --source-cells zone1 --tablet-types "rdonly,replica" --target-cells zone1 --update-table-stats --max-report-sample-rows 1000 --wait --wait-update-interval 5s --max-diff-duration 1h --row-diff-column-truncate-at 0`,
 		SilenceUsage:          true,
 		DisableFlagsInUseLine: true,
 		Aliases:               []string{"Create"},
@@ -199,8 +200,8 @@ vtctldclient --server localhost:15999 vdiff --workflow commerce2customer --targe
 	show = &cobra.Command{
 		Use:   "show",
 		Short: "Show the status of a VDiff.",
-		Example: `vtctldclient --server localhost:15999 vdiff --workflow commerce2customer --target-keyspace customer show last
-vtctldclient --server localhost:15999 vdiff --workflow commerce2customer --target-keyspace customer show a037a9e2-5628-11ee-8c99-0242ac120002
+		Example: `vtctldclient --server localhost:15999 vdiff --workflow commerce2customer --target-keyspace customer show last --verbose --format json
+vtctldclient --server :15999 vdiff --workflow commerce2customer --target-keyspace customer show a037a9e2-5628-11ee-8c99-0242ac120002
 vtctldclient --server localhost:15999 vdiff --workflow commerce2customer --target-keyspace customer show all`,
 		DisableFlagsInUseLine: true,
 		Aliases:               []string{"Show"},
@@ -294,6 +295,7 @@ func commandCreate(cmd *cobra.Command, args []string) error {
 		AutoRetry:                   createOptions.AutoRetry,
 		MaxReportSampleRows:         createOptions.MaxReportSampleRows,
 		MaxDiffDuration:             protoutil.DurationToProto(createOptions.MaxDiffDuration),
+		RowDiffColumnTruncateAt:     createOptions.RowDiffColumnTruncateAt,
 	})
 
 	if err != nil {
@@ -887,6 +889,7 @@ func registerCommands(root *cobra.Command) {
 	create.Flags().BoolVar(&createOptions.AutoRetry, "auto-retry", true, "Should this vdiff automatically retry and continue in case of recoverable errors.")
 	create.Flags().BoolVar(&createOptions.UpdateTableStats, "update-table-stats", false, "Update the table statistics, using ANALYZE TABLE, on each table involved in the VDiff during initialization. This will ensure that progress estimates are as accurate as possible -- but it does involve locks and can potentially impact query processing on the target keyspace.")
 	create.Flags().DurationVar(&createOptions.MaxDiffDuration, "max-diff-duration", 0, "How long should an individual table diff run before being stopped and restarted in order to lessen the impact on tablets due to holding open database snapshots for long periods of time (0 is the default and means no time limit).")
+	create.Flags().Int64Var(&createOptions.RowDiffColumnTruncateAt, "row-diff-column-truncate-at", 128, "When showing row differences, truncate the non Primary Key column values to this length. A value less than 1 means do not truncate.")
 	base.AddCommand(create)
 
 	base.AddCommand(delete)

go/test/endtoend/vreplication/cluster_test.go

@@ -471,9 +471,8 @@ func (vc *VitessCluster) AddKeyspace(t *testing.T, cells []*Cell, ksName string,
 		SidecarDBName: sidecarDBName,
 	}
 
-	if err := vc.VtctldClient.CreateKeyspace(keyspace.Name, keyspace.SidecarDBName); err != nil {
-		t.Fatal(err.Error())
-	}
+	err := vc.VtctldClient.CreateKeyspace(keyspace.Name, keyspace.SidecarDBName)
+	require.NoError(t, err)
 
 	log.Infof("Applying throttler config for keyspace %s", keyspace.Name)
 	req := &vtctldatapb.UpdateThrottlerConfigRequest{Enable: true, Threshold: throttlerConfig.Threshold, CustomQuery: throttlerConfig.Query}
@@ -497,15 +496,13 @@ func (vc *VitessCluster) AddKeyspace(t *testing.T, cells []*Cell, ksName string,
 
 	require.NoError(t, vc.AddShards(t, cells, keyspace, shards, numReplicas, numRdonly, tabletIDBase, opts))
 	if schema != "" {
-		if err := vc.VtctlClient.ApplySchema(ksName, schema); err != nil {
-			t.Fatal(err.Error())
-		}
+		err := vc.VtctlClient.ApplySchema(ksName, schema)
+		require.NoError(t, err)
 	}
 	keyspace.Schema = schema
 	if vschema != "" {
-		if err := vc.VtctlClient.ApplyVSchema(ksName, vschema); err != nil {
-			t.Fatal(err.Error())
-		}
+		err := vc.VtctlClient.ApplyVSchema(ksName, vschema)
+		require.NoError(t, err)
 	}
 	keyspace.VSchema = vschema
 
@@ -681,9 +678,8 @@ func (vc *VitessCluster) AddShards(t *testing.T, cells []*Cell, keyspace *Keyspa
 			}
 			for ind, tablet := range tablets {
 				log.Infof("Running Setup() for vttablet %s", tablets[ind].Name)
-				if err := tablet.Vttablet.Setup(); err != nil {
-					t.Fatal(err.Error())
-				}
+				err := tablet.Vttablet.Setup()
+				require.NoError(t, err)
 				// Set time_zone to UTC for all tablets. Without this it fails locally on some MacOS setups.
 				query := "SET GLOBAL time_zone = '+00:00';"
 				qr, err := tablet.Vttablet.QueryTablet(query, tablet.Vttablet.Keyspace, false)
@@ -781,9 +777,8 @@ func (vc *VitessCluster) StartVtgate(t testing.TB, cell *Cell, cellsToWatch stri
 		extraVTGateArgs,
 		vc.ClusterConfig.vtgatePlannerVersion)
 	require.NotNil(t, vtgate)
-	if err := vtgate.Setup(); err != nil {
-		t.Fatal(err.Error())
-	}
+	err := vtgate.Setup()
+	require.NoError(t, err)
 	cell.Vtgates = append(cell.Vtgates, vtgate)
 }
 

go/test/endtoend/vreplication/vdiff2_test.go

@@ -385,8 +385,9 @@ func testCLIFlagHandling(t *testing.T, targetKs, workflowName string, cell *Cell
 			TabletTypes: "replica,primary,rdonly",
 		},
 		ReportOptions: &tabletmanagerdatapb.VDiffReportOptions{
-			MaxSampleRows: 888,
-			OnlyPks:       true,
+			MaxSampleRows:           888,
+			OnlyPks:                 true,
+			RowDiffColumnTruncateAt: 444,
 		},
 	}
 
@@ -404,6 +405,7 @@ func testCLIFlagHandling(t *testing.T, targetKs, workflowName string, cell *Cell
 			fmt.Sprintf("--update-table-stats=%t", expectedOptions.CoreOptions.UpdateTableStats),
 			fmt.Sprintf("--auto-retry=%t", expectedOptions.CoreOptions.AutoRetry),
 			fmt.Sprintf("--only-pks=%t", expectedOptions.ReportOptions.OnlyPks),
+			fmt.Sprintf("--row-diff-column-truncate-at=%d", expectedOptions.ReportOptions.RowDiffColumnTruncateAt),
 			"--tablet-types-in-preference-order=false", // So tablet_types should not start with "in_order:", which is the default
 			"--format=json") // So we can easily grab the UUID
 		require.NoError(t, err, "vdiff command failed: %s", res)

go/vt/binlog/binlogplayer/mock_dbclient.go

@@ -103,13 +103,13 @@ func NewMockDbaClient(t *testing.T) *MockDBClient {
 // ExpectRequest adds an expected result to the mock.
 // This function should not be called conncurrently with other commands.
 func (dc *MockDBClient) ExpectRequest(query string, result *sqltypes.Result, err error) {
+	dc.expectMu.Lock()
+	defer dc.expectMu.Unlock()
 	select {
 	case <-dc.done:
 		dc.done = make(chan struct{})
 	default:
 	}
-	dc.expectMu.Lock()
-	defer dc.expectMu.Unlock()
 	dc.expect = append(dc.expect, &mockExpect{
 		query:  query,
 		result: result,
@@ -121,13 +121,13 @@ func (dc *MockDBClient) ExpectRequest(query string, result *sqltypes.Result, err
 // queryRE is a regular expression.
 // This function should not be called conncurrently with other commands.
 func (dc *MockDBClient) ExpectRequestRE(queryRE string, result *sqltypes.Result, err error) {
+	dc.expectMu.Lock()
+	defer dc.expectMu.Unlock()
 	select {
 	case <-dc.done:
 		dc.done = make(chan struct{})
 	default:
 	}
-	dc.expectMu.Lock()
-	defer dc.expectMu.Unlock()
 	dc.expect = append(dc.expect, &mockExpect{
 		query:  queryRE,
 		re:     regexp.MustCompile(queryRE),