Comment the VDiffCreate proto

And enforce sensible defaults on the server side.

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

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

@@ -417,12 +417,14 @@ type summary struct {
 	RowsCompared       int64
 	HasMismatch        bool
 	Shards             string
-	StartedAt          string                                 `json:"StartedAt,omitempty"`
-	CompletedAt        string                                 `json:"CompletedAt,omitempty"`
-	TableSummaryMap    map[string]tableSummary                `json:"TableSummary,omitempty"`
-	Reports            map[string]map[string]vdiff.DiffReport `json:"Reports,omitempty"`
-	Errors             map[string]string                      `json:"Errors,omitempty"`
-	Progress           *vdiff.ProgressReport                  `json:"Progress,omitempty"`
+	StartedAt          string                  `json:"StartedAt,omitempty"`
+	CompletedAt        string                  `json:"CompletedAt,omitempty"`
+	TableSummaryMap    map[string]tableSummary `json:"TableSummary,omitempty"`
+	// This is keyed by table name and then by shard name.
+	Reports map[string]map[string]vdiff.DiffReport `json:"Reports,omitempty"`
+	// This is keyed by shard name.
+	Errors   map[string]string     `json:"Errors,omitempty"`
+	Progress *vdiff.ProgressReport `json:"Progress,omitempty"`
 }
 
 const summaryTextTemplate = `
@@ -892,7 +894,7 @@ func registerCommands(root *cobra.Command) {
 	create.Flags().Int64Var(&createOptions.Limit, "limit", math.MaxInt64, "Max rows to stop comparing after.")
 	create.Flags().BoolVar(&createOptions.DebugQuery, "debug-query", false, "Adds a mysql query to the report that can be used for further debugging.")
 	create.Flags().Int64Var(&createOptions.MaxReportSampleRows, "max-report-sample-rows", 10, "Maximum number of row differences to report (0 for all differences). NOTE: when increasing this value it is highly recommended to also specify --only-pks")
-	create.Flags().BoolVar(&createOptions.OnlyPKs, "only-pks", false, "When reporting missing rows, only show primary keys in the report.")
+	create.Flags().BoolVar(&createOptions.OnlyPKs, "only-pks", false, "When reporting row differences, only show primary keys in the report.")
 	create.Flags().StringSliceVar(&createOptions.Tables, "tables", nil, "Only run vdiff for these tables in the workflow.")
 	create.Flags().Int64Var(&createOptions.MaxExtraRowsToCompare, "max-extra-rows-to-compare", 1000, "If there are collation differences between the source and target, you can have rows that are identical but simply returned in a different order from MySQL. We will do a second pass to compare the rows for any actual differences in this case and this flag allows you to control the resources used for this operation.")
 	create.Flags().BoolVar(&createOptions.Wait, "wait", false, "When creating or resuming a vdiff, wait for it to finish before exiting.")

go/vt/vtctl/workflow/server.go

@@ -1850,6 +1850,9 @@ func (s *Server) VDiffCreate(ctx context.Context, req *vtctldatapb.VDiffCreateRe
 
 	tabletTypesStr := discovery.BuildTabletTypesString(req.TabletTypes, req.TabletSelectionPreference)
 
+	if req.Limit == 0 { // This would produce no useful results
+		req.Limit = math.MaxInt64
+	}
 	// This is a pointer so there's no ZeroValue in the message
 	// and an older v18 client will not provide it.
 	if req.MaxDiffDuration == nil {
@@ -1859,7 +1862,10 @@ func (s *Server) VDiffCreate(ctx context.Context, req *vtctldatapb.VDiffCreateRe
 	// client should always provide them, but we check anyway to
 	// be safe.
 	if req.FilteredReplicationWaitTime == nil {
-		req.FilteredReplicationWaitTime = &vttimepb.Duration{}
+		// A value of 0 is not valid as the vdiff will never succeed.
+		req.FilteredReplicationWaitTime = &vttimepb.Duration{
+			Seconds: int64(DefaultTimeout.Seconds()),
+		}
 	}
 	if req.WaitUpdateInterval == nil {
 		req.WaitUpdateInterval = &vttimepb.Duration{}

proto/vtctldata.proto

@@ -1898,25 +1898,72 @@ message ValidateVSchemaResponse {
 message VDiffCreateRequest {
   string workflow = 1;
   string target_keyspace = 2;
+  // A unique identifier for the vdiff. If empty, a new UUID will be generated.
   string uuid = 3;
+  // The cells to look for source tablets in. If empty, all cells are used.
   repeated string source_cells = 4;
+  // The cells to look for target tablets in. If empty, all cells are used.
   repeated string target_cells = 5;
+  // The tablet types to use when searching for tablets to use when streaming results.
+  // A default value of "replica,rdonly,primary" is used by the tablet picker.
   repeated topodata.TabletType tablet_types = 6;
+  // When performing source tablet selection, look for candidates in the type order
+  // as they are listed in the tablet_types value (or the default of
+  // "replica,rdonly,primary" that the tablet picker uses). The default is
+  // ANY (0) and you can use INORDER (1) to e.g. ensure that a primary table is
+  // only used if there are no available replica or rdonly tablets.
   tabletmanagerdata.TabletSelectionPreference tablet_selection_preference = 7;
+  // The tables to compare. If empty, all tables in the workflow are compared.
   repeated string tables = 8;
+  // The maximum number of rows to compare for each table on each shard.
+  // The default is a max int64 value: 2^63 - 1 or 9,223,372,036,854,775,807.
   int64 limit = 9;
+  // How long to wait for vreplication to catch up when attempting to setup the
+  // table snapshots on the souce and target to use for the diff. The default
+  // is 30s.
   vttime.Duration filtered_replication_wait_time = 10;
+  // Include the MySQL query used for the diff in the report that is stored on
+  // each shard primary tablet in the _vt.vdiff_table records.
   bool debug_query = 11;
+  // Only show the Primary Key columns in any row diff output. You would
+  // typically want to use this if you set the max_sample_rows very high.
   bool only_p_ks = 12;
+  // Update the table statistics, using ANALYZE TABLE, on each table involved
+  // in the vdiff during initialization on each target shard. 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.
   bool update_table_stats = 13;
+  // If there are collation differences between the source and target, you can
+  // have rows that are identical but simply returned in a different order from
+  // MySQL. We will do a second pass to compare the rows for any actual
+  // differences in this case and this flag allows you to control the resources
+  // used for this operation.
   int64 max_extra_rows_to_compare = 14;
+  // Wait for the vdiff to complete before returning (making the call synchronous
+  // vs asynchronous by default).
   bool wait = 15;
+  // When wait is true, this is how frequently the vdiff progress will be shown.
   vttime.Duration wait_update_interval = 16;
+  // Automatically retry the vdiff if we encounter an error. This should almost
+  // always be set to true (default is false).
   bool auto_retry = 17;
+  // Include additional information in the vdiff report that is produced and stored
+  // on each target shard primary's _vt sidecar database.
   bool verbose = 18;
+  // The maximum number of rows to include in the row diff report (when differences
+  // are found) for each table on each shard. The default is 0.
   int64 max_report_sample_rows = 19;
+  // The maximum time that a diff of a single table can run on each target shard
+  // before it is stopped and then later resumed from where we left off. This can
+  // be helpful in limiting the impact of holding open that large transaction where
+  // we scan up to every row in the table.
+  // The default is 0 or no limit.
   vttime.Duration max_diff_duration = 20;
+  // At what length should we truncate the column values in the row diff report
+  // generated for each table on each shard when differences are detected.
   int64 row_diff_column_truncate_at = 21;
+  // Auto start the vdiff after creating it. The default is true if no value is
+  // specified.
   optional bool auto_start = 22;
 }