Add GenerateShardRanges to vtctl commands#6752
Conversation
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
rafael
left a comment
rafael
left a comment
There was a problem hiding this comment.
From my perspective this is a great idea to have a first class citizen. I've seen this come up multiple times in Slack where people it's not super sure about how to generate the shard ranges for a shard.
After thinking a bit more about this, we could also add this script to the contrib one. Personally, I think it makes more sense here.
@deepthi what do you think about this?
go/vt/vtctl/vtctl.go
Outdated
| {"ListTablets", commandListTablets, | ||
| "<tablet alias> ...", | ||
| "Lists specified tablets in an awk-friendly way."}, | ||
| {"ListShardRanges", commandListShardRanges, |
There was a problem hiding this comment.
nit - Maybe "GenerateShardRanges" ? I think List reads as something that already exists.
I think it's a great idea :) +1 on keeping it in the main repo and not in contrib. |
go/vt/vtctl/vtctl_test.go
Outdated
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got, err := listShardRanges(tt.args.shards) | ||
| if (err != nil) != tt.wantErr { |
There was a problem hiding this comment.
May I suggest replacing the more verbose comparisons with assert and require?
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
ajm188
changed the title
go/vt/vtctl/vtctl_test.go
Outdated
| func TestShardCalculatorForShardsGreaterThan512(t *testing.T) { | ||
| got, err := generateShardRanges(512) | ||
| if err != nil { | ||
| t.Errorf("listShardRanges() error = %v", err) |
Signed-off-by: Andrew Mason <amason@slack-corp.com>
| start := 0 | ||
| end := 0 | ||
|
|
||
| // If shards does not divide evenly into maxShards, then there is some lossiness, |
There was a problem hiding this comment.
Nice implementation and documentation 👍
4 participants
Resolves #6751