Refactor and cleanup treatment of keyspace IDs and KeyRange (#12524)

* 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]> --------- Signed-off-by: Jeremy Cole <[email protected]> Signed-off-by: Hormoz Kheradmand <[email protected]> Co-authored-by: Hormoz Kheradmand <[email protected]>
vitessio · Mar 24, 2023 · 47f7234 · 47f7234
1 parent 15f491e
commit 47f7234
Show file tree

Hide file tree

Showing 18 changed files with 1,187 additions and 400 deletions.
diff --git a/go/test/endtoend/keyspace/keyspace_test.go b/go/test/endtoend/keyspace/keyspace_test.go
@@ -17,14 +17,15 @@ limitations under the License.
 package sequence
 
 import (
-	"bytes"
 	"encoding/binary"
 	"encoding/json"
 	"flag"
 	"os"
 	"strings"
 	"testing"
 
+	"vitess.io/vitess/go/vt/key"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -392,8 +393,8 @@ func TestKeyspaceToShardName(t *testing.T) {
 				shardKIDs := shardKIdMap[shardRef.Name]
 				for _, kid := range shardKIDs {
 					id = packKeyspaceID(kid)
-					assert.True(t, bytes.Compare(shardRef.KeyRange.Start, id) <= 0 &&
-						(len(shardRef.KeyRange.End) == 0 || bytes.Compare(id, shardRef.KeyRange.End) < 0))
+					assert.True(t, key.Compare(shardRef.KeyRange.Start, id) <= 0 &&
+						(key.Empty(shardRef.KeyRange.End) || key.Compare(id, shardRef.KeyRange.End) < 0))
 				}
 			}
 		}

diff --git a/go/vt/discovery/topology_watcher.go b/go/vt/discovery/topology_watcher.go
@@ -362,7 +362,7 @@ func (fbs *FilterByShard) IsIncluded(tablet *topodata.Tablet) bool {
 			// Exact match (probably a non-sharded keyspace).
 			return true
 		}
-		if kr != nil && c.keyRange != nil && key.KeyRangeIncludes(c.keyRange, kr) {
+		if kr != nil && c.keyRange != nil && key.KeyRangeContainsKeyRange(c.keyRange, kr) {
 			// Our filter's KeyRange includes the provided KeyRange
 			return true
 		}

diff --git a/go/vt/key/destination.go b/go/vt/key/destination.go
@@ -128,7 +128,7 @@ func (d DestinationExactKeyRange) String() string {
 
 func processExactKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb.KeyRange, addShard func(shard string) error) error {
 	sort.SliceStable(allShards, func(i, j int) bool {
-		return KeyRangeStartSmaller(allShards[i].GetKeyRange(), allShards[j].GetKeyRange())
+		return KeyRangeLess(allShards[i].GetKeyRange(), allShards[j].GetKeyRange())
 	})
 
 	shardnum := 0
@@ -139,7 +139,7 @@ func processExactKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb
 		shardnum++
 	}
 	for shardnum < len(allShards) {
-		if !KeyRangesIntersect(kr, allShards[shardnum].KeyRange) {
+		if !KeyRangeIntersect(kr, allShards[shardnum].KeyRange) {
 			// If we are over the requested keyrange, we
 			// can stop now, we won't find more.
 			break
@@ -215,7 +215,7 @@ func (d DestinationKeyRange) String() string {
 
 func processKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb.KeyRange, addShard func(shard string) error) error {
 	for _, shard := range allShards {
-		if !KeyRangesIntersect(kr, shard.KeyRange) {
+		if !KeyRangeIntersect(kr, shard.KeyRange) {
 			// We don't need that shard.
 			continue
 		}