Skip to content

Commit 230340b

Browse files
jeremycoleshanth96
authored andcommitted
Refactor and cleanup treatment of keyspace IDs and KeyRange
Signed-off-by: Jeremy Cole <[email protected]> Address internal review comments Signed-off-by: Jeremy Cole <[email protected]> Fix apparent bug in KeyRangeContiguous when a or b are full-range Signed-off-by: Jeremy Cole <[email protected]> Add test for bug in comparing "0003" vs "000300" Signed-off-by: Hormoz Kheradmand <[email protected]> Remove trailing zeroes in key.Normalize instead of adding padding Signed-off-by: Jeremy Cole <[email protected]> Address review feedback; test formatting, comments, function naming Signed-off-by: Jeremy Cole <[email protected]> Refactor tests for TestKeyRangesIntersect Signed-off-by: Jeremy Cole <[email protected]> Rename KeyRangesIntersect to KeyRangeIntersect for consistency Signed-off-by: Jeremy Cole <[email protected]> Remove unused KeyRangesOverlap function Signed-off-by: Jeremy Cole <[email protected]> Rename KeyRangeIncludes to KeyRangeContainsKeyRange, clean up and add tests Signed-off-by: Jeremy Cole <[email protected]>
1 parent 652b0da commit 230340b

File tree

18 files changed

+1187
-400
lines changed

18 files changed

+1187
-400
lines changed

go/test/endtoend/keyspace/keyspace_test.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@ limitations under the License.
1717
package sequence
1818

1919
import (
20-
"bytes"
2120
"encoding/binary"
2221
"encoding/json"
2322
"flag"
2423
"os"
2524
"strings"
2625
"testing"
2726

27+
"vitess.io/vitess/go/vt/key"
28+
2829
"github.com/stretchr/testify/assert"
2930
"github.com/stretchr/testify/require"
3031

@@ -392,8 +393,8 @@ func TestKeyspaceToShardName(t *testing.T) {
392393
shardKIDs := shardKIdMap[shardRef.Name]
393394
for _, kid := range shardKIDs {
394395
id = packKeyspaceID(kid)
395-
assert.True(t, bytes.Compare(shardRef.KeyRange.Start, id) <= 0 &&
396-
(len(shardRef.KeyRange.End) == 0 || bytes.Compare(id, shardRef.KeyRange.End) < 0))
396+
assert.True(t, key.Compare(shardRef.KeyRange.Start, id) <= 0 &&
397+
(key.Empty(shardRef.KeyRange.End) || key.Compare(id, shardRef.KeyRange.End) < 0))
397398
}
398399
}
399400
}

go/vt/discovery/topology_watcher.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ func (fbs *FilterByShard) IsIncluded(tablet *topodata.Tablet) bool {
362362
// Exact match (probably a non-sharded keyspace).
363363
return true
364364
}
365-
if kr != nil && c.keyRange != nil && key.KeyRangeIncludes(c.keyRange, kr) {
365+
if kr != nil && c.keyRange != nil && key.KeyRangeContainsKeyRange(c.keyRange, kr) {
366366
// Our filter's KeyRange includes the provided KeyRange
367367
return true
368368
}

go/vt/key/destination.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func (d DestinationExactKeyRange) String() string {
128128

129129
func processExactKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb.KeyRange, addShard func(shard string) error) error {
130130
sort.SliceStable(allShards, func(i, j int) bool {
131-
return KeyRangeStartSmaller(allShards[i].GetKeyRange(), allShards[j].GetKeyRange())
131+
return KeyRangeLess(allShards[i].GetKeyRange(), allShards[j].GetKeyRange())
132132
})
133133

134134
shardnum := 0
@@ -139,7 +139,7 @@ func processExactKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb
139139
shardnum++
140140
}
141141
for shardnum < len(allShards) {
142-
if !KeyRangesIntersect(kr, allShards[shardnum].KeyRange) {
142+
if !KeyRangeIntersect(kr, allShards[shardnum].KeyRange) {
143143
// If we are over the requested keyrange, we
144144
// can stop now, we won't find more.
145145
break
@@ -215,7 +215,7 @@ func (d DestinationKeyRange) String() string {
215215

216216
func processKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb.KeyRange, addShard func(shard string) error) error {
217217
for _, shard := range allShards {
218-
if !KeyRangesIntersect(kr, shard.KeyRange) {
218+
if !KeyRangeIntersect(kr, shard.KeyRange) {
219219
// We don't need that shard.
220220
continue
221221
}

0 commit comments

Comments
 (0)