Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions go/cmd/vtctldclient/command/vreplication/vdiff/vdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ var (
}{}

showOptions = struct {
Arg string
Verbose bool
Arg string
Verbose bool
NoSamples bool
}{}

stopOptions = struct {
Expand Down Expand Up @@ -318,10 +319,17 @@ func commandCreate(cmd *cobra.Command, args []string) error {
case <-ctx.Done():
return vterrors.Errorf(vtrpcpb.Code_CANCELED, "context has expired")
case <-tkr.C:
// This is a progress poll that only renders a single non-verbose
// summary (state, counts, has_mismatch), so request no_samples:
Comment thread
pedroalb marked this conversation as resolved.
Outdated
// the per-row sample arrays are never shown here and fetching them
// on every interval can push the aggregated response past the gRPC
// message limit for large diffs, killing the wait. The authoritative
// samples remain available via `vdiff show --verbose`.
resp, err := vtctldClient.VDiffShow(ctx, &vtctldatapb.VDiffShowRequest{
Workflow: common.BaseOptions.Workflow,
TargetKeyspace: common.BaseOptions.TargetKeyspace,
Arg: uuidStr,
NoSamples: true,
Comment thread
pedroalb marked this conversation as resolved.
})
if err != nil {
return err
Expand Down Expand Up @@ -645,6 +653,7 @@ func commandShow(cmd *cobra.Command, args []string) error {
Workflow: common.BaseOptions.Workflow,
TargetKeyspace: common.BaseOptions.TargetKeyspace,
Arg: showOptions.Arg,
NoSamples: showOptions.NoSamples,
})
if err != nil {
return err
Expand Down Expand Up @@ -709,6 +718,7 @@ func registerCommands(root *cobra.Command) {
base.AddCommand(resume)

show.Flags().BoolVar(&showOptions.Verbose, "verbose", false, "Show verbose output in summaries")
show.Flags().BoolVar(&showOptions.NoSamples, "no-samples", false, "Strip the per-table diff report's row-sample arrays (keeping the scalar counters). Useful for large diffs where the samples can exceed gRPC message limits.")
Comment thread
pedroalb marked this conversation as resolved.
Comment thread
pedroalb marked this conversation as resolved.
Comment thread
pedroalb marked this conversation as resolved.
base.AddCommand(show)

stop.Flags().StringSliceVar(&stopOptions.TargetShards, "target-shards", nil, "The target shards to stop the vdiff on; default is all shards.")
Expand Down
18 changes: 18 additions & 0 deletions go/cmd/vtctldclient/command/vreplication/vdiff/vdiff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/google/uuid"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/sqltypes"
Expand Down Expand Up @@ -813,3 +814,20 @@ func TestGetStructNames(t *testing.T) {
want := []string{"A", "B"}
require.Equal(t, want, got)
}

// TestShowNoSamplesFlag guards the `vdiff show --no-samples` flag against
// accidental removal or renaming. The go/flags/endtoend golden fixtures only
// cover each binary's root --help, not subcommands, so this is the flag's
// regression coverage (mirrors the movetables keep-data flag test).
Comment thread
pedroalb marked this conversation as resolved.
Outdated
func TestShowNoSamplesFlag(t *testing.T) {
root := &cobra.Command{Use: "test"}
registerCommands(root)

showCmd, _, err := root.Find([]string{"VDiff", "show"})
require.NoError(t, err)

flag := showCmd.Flags().Lookup("no-samples")
require.NotNil(t, flag, "vdiff show must expose the --no-samples flag")
require.Equal(t, "false", flag.DefValue, "--no-samples must default to false")
require.Contains(t, flag.Usage, "gRPC message limits")
}
21 changes: 17 additions & 4 deletions go/vt/proto/tabletmanagerdata/tabletmanagerdata.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions go/vt/proto/tabletmanagerdata/tabletmanagerdata_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 16 additions & 3 deletions go/vt/proto/vtctldata/vtctldata.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions go/vt/proto/vtctldata/vtctldata_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions go/vt/vtctl/workflow/vdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,11 @@ func (s *Server) VDiffShow(ctx context.Context, req *vtctldatapb.VDiffShowReques
Workflow: req.Workflow,
Action: string(vdiff.ShowAction),
ActionArg: req.Arg,
Options: &tabletmanagerdatapb.VDiffOptions{
ReportOptions: &tabletmanagerdatapb.VDiffReportOptions{
NoSamples: req.GetNoSamples(),
},
},
}

ts, err := s.buildTrafficSwitcher(ctx, req.TargetKeyspace, req.Workflow)
Expand Down
84 changes: 84 additions & 0 deletions go/vt/vtctl/workflow/vdiff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,3 +754,87 @@ func TestVDiffDelete(t *testing.T) {
})
}
}

// TestVDiffShow verifies that VDiffShow forwards the no_samples flag from the
// vtctld request through to the tabletmanager VDiff request sent to every target
// primary, for both flag values. This exercises the vtctld->tabletmanager
// plumbing so a regression there is caught rather than relying on query-string
// comparisons alone.
func TestVDiffShow(t *testing.T) {
ctx := t.Context()
sourceKeyspace := &testKeyspace{
KeyspaceName: "sourceks",
ShardNames: []string{"0"},
}
targetKeyspace := &testKeyspace{
KeyspaceName: "targetks",
ShardNames: []string{"-80", "80-"},
}
workflow := "testwf"
uuid := uuid.New().String()
env := newTestEnv(t, ctx, defaultCellName, sourceKeyspace, targetKeyspace)
t.Cleanup(env.close)

env.tmc.strict = true
action := string(vdiff.ShowAction)

// expectedShowRequest is the tabletmanager request VDiffShow must forward to
// each target primary for a given no_samples value.
expectedShowRequest := func(noSamples bool) *tabletmanagerdatapb.VDiffRequest {
return &tabletmanagerdatapb.VDiffRequest{
Keyspace: targetKeyspace.KeyspaceName,
Workflow: workflow,
Action: action,
ActionArg: uuid,
Options: &tabletmanagerdatapb.VDiffOptions{
ReportOptions: &tabletmanagerdatapb.VDiffReportOptions{
NoSamples: noSamples,
},
},
}
}
// bothTargets expects the same forwarded request on both target shards.
bothTargets := func(noSamples bool) map[*topodatapb.Tablet]*vdiffRequestResponse {
return map[*topodatapb.Tablet]*vdiffRequestResponse{
env.tablets[targetKeyspace.KeyspaceName][startingTargetTabletUID]: {req: expectedShowRequest(noSamples)},
env.tablets[targetKeyspace.KeyspaceName][startingTargetTabletUID+tabletUIDStep]: {req: expectedShowRequest(noSamples)},
}
}

tests := []struct {
name string
req *vtctldatapb.VDiffShowRequest
expectedVDiffRequests map[*topodatapb.Tablet]*vdiffRequestResponse
}{
{
name: "default forwards no_samples=false",
req: &vtctldatapb.VDiffShowRequest{
TargetKeyspace: targetKeyspace.KeyspaceName,
Workflow: workflow,
Arg: uuid,
},
expectedVDiffRequests: bothTargets(false),
},
{
name: "no_samples forwards no_samples=true",
req: &vtctldatapb.VDiffShowRequest{
TargetKeyspace: targetKeyspace.KeyspaceName,
Workflow: workflow,
Arg: uuid,
NoSamples: true,
},
expectedVDiffRequests: bothTargets(true),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for tab, vdr := range tt.expectedVDiffRequests {
env.tmc.expectVDiffRequest(tab, vdr)
}
got, err := env.ws.VDiffShow(ctx, tt.req)
require.NoError(t, err)
require.NotNil(t, got)
env.tmc.confirmVDiffRequests(t)
})
}
}
Loading