Skip to content

Commit

Permalink
VDiff: Improve row diff handling in report (vitessio#16618)
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Lord <[email protected]>
  • Loading branch information
mattlord authored and venkatraju committed Aug 29, 2024
1 parent dc7a48c commit 737115d
Show file tree
Hide file tree
Showing 19 changed files with 993 additions and 630 deletions.
9 changes: 6 additions & 3 deletions go/cmd/vtctldclient/command/vreplication/vdiff/vdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ var (
WaitUpdateInterval time.Duration
AutoRetry bool
MaxDiffDuration time.Duration
RowDiffColumnTruncateAt int64
}{}

deleteOptions = struct {
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
25 changes: 10 additions & 15 deletions go/test/endtoend/vreplication/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 4 additions & 2 deletions go/test/endtoend/vreplication/vdiff2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}

Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions go/vt/binlog/binlogplayer/mock_dbclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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),
Expand Down
Loading

0 comments on commit 737115d

Please sign in to comment.