Skip to content

Commit

Permalink
Changes from further testing
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Lord <mattalord@gmail.com>
  • Loading branch information
mattlord committed Jan 2, 2025
1 parent f0047fa commit 24fdf41
Showing 1 changed file with 9 additions and 36 deletions.
45 changes: 9 additions & 36 deletions go/cmd/vtctldclient/command/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ var copySchemaShardOptions = struct {
}{}

func commandCopySchemaShard(cmd *cobra.Command, args []string) error {
cli.FinishedParsing(cmd)

destKeyspace, destShard, err := topoproto.ParseKeyspaceShard(cmd.Flags().Arg(1))
if err != nil {
return err
Expand Down Expand Up @@ -384,45 +386,16 @@ func commandValidateSchemaKeyspace(cmd *cobra.Command, args []string) error {
IncludeViews: validateSchemaKeyspaceOptions.IncludeViews,
})

// The client behavior here matches the behavior and output of the legacy vtctl[client].
// The way that the server side is written, the error is only set if an error is encountered
// performing the work. If there are any differences found then they are included in the
// results (no result is generated when the schemas match across the two tablets). It's also
// assumed that each tablet has the same differences with the reference/primary tablet and
// thus we only return the first result to match the behavior of the legacy vtctl[client]
// since the server side results also do not mention WHICH tablet differed from the
// reference/primary tablet (101 in the example below, while the tablets that had a
// difference which led to the result are 100 and 102). Here's an example full response:
// ❯ vtctldclient ValidateSchemaKeyspace commerce
// {
// "results": [
// "zone1-0000000101 has an extra table named mlord",
// "zone1-0000000101 has an extra table named mlord"
// ],
// "results_by_shard": {
// "0": {
// "results": [
// "zone1-0000000101 has an extra table named mlord",
// "zone1-0000000101 has an extra table named mlord"
// ]
// }
// }
// }
// TODO: This command, both client and server side, needs to be refactored:
// 1. We should always show the full results whether a difference is detected or not and
// leave the error handling unchanged, OR we should only show the user any results when
// there's a difference detected -- capturing that in the error on the server side and
// then passing that on to the user as an error.
// 2. The results/error should clearly note which tablet(s) are involved. In the example
// above the results are from zone1-0000000100 and zone1-0000000102 which differed
// from zone1-0000000101, but that is not clearly reflected in the output.
if err != nil {
return err
}
if len(resp.Results) > 0 {
return fmt.Errorf("%s", resp.Results[0])

data, err := cli.MarshalJSON(resp.ResultsByShard)
if err != nil {
return err
}
// No errors or mismatches found.

fmt.Printf("%s\n", data)
return nil
}

Expand All @@ -447,7 +420,7 @@ func commandValidateSchemaShard(cmd *cobra.Command, args []string) error {
return err
}

data, err := cli.MarshalJSON(resp)
data, err := cli.MarshalJSON(resp.Results)
if err != nil {
return err
}
Expand Down

0 comments on commit 24fdf41

Please sign in to comment.