Merge pull request vitessio#8296 from tinyspeck/add-padding-to-keyran…

…ge-comparison

Adds padding to keyrange comparison

go/vt/key/key.go

@@ -191,8 +191,30 @@ func KeyRangeEqual(left, right *topodatapb.KeyRange) bool {
  if right == nil {
  return len(left.Start) == 0 && len(left.End) == 0
  }
- return bytes.Equal(left.Start, right.Start) &&
- bytes.Equal(left.End, right.End)
+ return bytes.Equal(addPadding(left.Start), addPadding(right.Start)) &&
+ bytes.Equal(addPadding(left.End), addPadding(right.End))
+}
+
+// addPadding adds padding to make sure keyrange represents an 8 byte integer.
+// From Vitess docs:
+// A hash vindex produces an 8-byte number.
+// This means that all numbers less than 0x8000000000000000 will fall in shard -80.
+// Any number with the highest bit set will be >= 0x8000000000000000, and will therefore
+// belong to shard 80-.
+// This means that from a keyrange perspective -80 == 00-80 == 0000-8000 == 000000-800000
+// If we don't add this padding, we could run into issues when transitioning from keyranges
+// that use 2 bytes to 4 bytes.
+func addPadding(kr []byte) []byte {
+ paddedKr := make([]byte, 8)
+
+ for i := 0; i < len(kr); i++ {
+ paddedKr = append(paddedKr, kr[i])
+ }
+
+ for i := len(kr); i < 8; i++ {
+ paddedKr = append(paddedKr, 0)
+ }
+ return paddedKr
 }
 
 // KeyRangeStartSmaller returns true if right's keyrange start is _after_ left's start
@@ -214,7 +236,7 @@ func KeyRangeStartEqual(left, right *topodatapb.KeyRange) bool {
  if right == nil {
  return len(left.Start) == 0
  }
- return bytes.Equal(left.Start, right.Start)
+ return bytes.Equal(addPadding(left.Start), addPadding(right.Start))
 }
 
 // KeyRangeEndEqual returns true if both key ranges have the same end
@@ -225,7 +247,7 @@ func KeyRangeEndEqual(left, right *topodatapb.KeyRange) bool {
  if right == nil {
  return len(left.End) == 0
  }
- return bytes.Equal(left.End, right.End)
+ return bytes.Equal(addPadding(left.End), addPadding(right.End))
 }
 
 // For more info on the following functions, see:

go/vt/key/key_test.go

@@ -240,6 +240,167 @@ func TestKeyRangeAdd(t *testing.T) {
  }
 }
 
+func TestKeyRangeEndEqual(t *testing.T) {
+ testcases := []struct {
+ first string
+ second string
+ out bool
+ }{{
+ first: "",
+ second: "",
+ out: true,
+ }, {
+ first: "",
+ second: "-80",
+ out: false,
+ }, {
+ first: "40-",
+ second: "10-",
+ out: true,
+ }, {
+ first: "-8000",
+ second: "-80",
+ out: true,
+ }, {
+ first: "-8000",
+ second: "-8000000000000000",
+ out: true,
+ }, {
+ first: "-80",
+ second: "-8000",
+ out: true,
+ }}
+ stringToKeyRange := func(spec string) *topodatapb.KeyRange {
+ if spec == "" {
+ return nil
+ }
+ parts := strings.Split(spec, "-")
+ if len(parts) != 2 {
+ panic("invalid spec")
+ }
+ kr, err := ParseKeyRangeParts(parts[0], parts[1])
+ if err != nil {
+ panic(err)
+ }
+ return kr
+ }
+
+ for _, tcase := range testcases {
+ first := stringToKeyRange(tcase.first)
+ second := stringToKeyRange(tcase.second)
+ out := KeyRangeEndEqual(first, second)
+ if out != tcase.out {
+ t.Fatalf("KeyRangeEndEqual(%q, %q) expected %t, got %t", tcase.first, tcase.second, tcase.out, out)
+ }
+ }
+}
+
+func TestKeyRangeStartEqual(t *testing.T) {
+ testcases := []struct {
+ first string
+ second string
+ out bool
+ }{{
+ first: "",
+ second: "",
+ out: true,
+ }, {
+ first: "",
+ second: "-80",
+ out: true,
+ }, {
+ first: "40-",
+ second: "20-",
+ out: false,
+ }, {
+ first: "-8000",
+ second: "-80",
+ out: true,
+ }, {
+ first: "-8000",
+ second: "-8000000000000000",
+ out: true,
+ }, {
+ first: "-80",
+ second: "-8000",
+ out: true,
+ }}
+ stringToKeyRange := func(spec string) *topodatapb.KeyRange {
+ if spec == "" {
+ return nil
+ }
+ parts := strings.Split(spec, "-")
+ if len(parts) != 2 {
+ panic("invalid spec")
+ }
+ kr, err := ParseKeyRangeParts(parts[0], parts[1])
+ if err != nil {
+ panic(err)
+ }
+ return kr
+ }
+
+ for _, tcase := range testcases {
+ first := stringToKeyRange(tcase.first)
+ second := stringToKeyRange(tcase.second)
+ out := KeyRangeStartEqual(first, second)
+ if out != tcase.out {
+ t.Fatalf("KeyRangeStartEqual(%q, %q) expected %t, got %t", tcase.first, tcase.second, tcase.out, out)
+ }
+ }
+}
+
+func TestKeyRangeEqual(t *testing.T) {
+ testcases := []struct {
+ first string
+ second string
+ out bool
+ }{{
+ first: "",
+ second: "",
+ out: true,
+ }, {
+ first: "",
+ second: "-80",
+ out: false,
+ }, {
+ first: "-8000",
+ second: "-80",
+ out: true,
+ }, {
+ first: "-8000",
+ second: "-8000000000000000",
+ out: true,
+ }, {
+ first: "-80",
+ second: "-8000",
+ out: true,
+ }}
+ stringToKeyRange := func(spec string) *topodatapb.KeyRange {
+ if spec == "" {
+ return nil
+ }
+ parts := strings.Split(spec, "-")
+ if len(parts) != 2 {
+ panic("invalid spec")
+ }
+ kr, err := ParseKeyRangeParts(parts[0], parts[1])
+ if err != nil {
+ panic(err)
+ }
+ return kr
+ }
+
+ for _, tcase := range testcases {
+ first := stringToKeyRange(tcase.first)
+ second := stringToKeyRange(tcase.second)
+ out := KeyRangeEqual(first, second)
+ if out != tcase.out {
+ t.Fatalf("KeyRangeEqual(%q, %q) expected %t, got %t", tcase.first, tcase.second, tcase.out, out)
+ }
+ }
+}
+
 func TestEvenShardsKeyRange_Error(t *testing.T) {
  testCases := []struct {
  i, n int

go/vt/topotools/split_test.go

@@ -105,6 +105,18 @@ func TestValidateForReshard(t *testing.T) {
  sources: []string{"-80", "80-"},
  targets: []string{"-40", "40-"},
  out: "",
+ }, {
+ sources: []string{"52-53"},
+ targets: []string{"5200-5240", "5240-5280", "5280-52c0", "52c0-5300"},
+ out: "",
+ }, {
+ sources: []string{"5200-5300"},
+ targets: []string{"520000-524000", "524000-528000", "528000-52c000", "52c000-530000"},
+ out: "",
+ }, {
+ sources: []string{"-80", "80-"},
+ targets: []string{"-4000000000000000", "4000000000000000-8000000000000000", "8000000000000000-80c0000000000000", "80c0000000000000-"},
+ out: "",
  }, {
  sources: []string{"80-", "-80"},
  targets: []string{"-40", "40-"},