Skip to content

Commit

Permalink
kv: stop encoding synthetic timestamp bit in mvcc keys
Browse files Browse the repository at this point in the history
Informs #101938.

This commit removes logic in mvcc key encoding routines that handle
synthetic timestamps. As a result, we no longer write keys with
synthetic timestamps, though we retain the ability to decode them.

As described in #72121 (comment)
and later in 24c56df
(see "Future improvements"), the introduction of the mvcc value header
and the optional, per-version local timestamp paved the way for the
removal of synthetic timestamps. MVCC keys no longer need to carry the
synthetic bit in order for reads from GLOBAL tables to behave properly.
As a result, we no longer need to write it.

Release note: None
  • Loading branch information
nvanbenschoten committed Jun 25, 2023
1 parent a2c2c06 commit f05520f
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 3,099 deletions.
1 change: 0 additions & 1 deletion pkg/storage/batch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,6 @@ func TestDecodeKey(t *testing.T) {
{Key: []byte("foo")},
{Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1}},
{Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1, Logical: 1}},
{Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1, Logical: 1, Synthetic: true}},
}
for _, test := range tests {
t.Run(test.String(), func(t *testing.T) {
Expand Down
29 changes: 27 additions & 2 deletions pkg/storage/engine_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package storage

import (
"encoding/hex"
"fmt"
"math/rand"
"testing"
Expand Down Expand Up @@ -88,7 +89,6 @@ func TestMVCCAndEngineKeyEncodeDecode(t *testing.T) {
{key: MVCCKey{Key: roachpb.Key("a")}},
{key: MVCCKey{Key: roachpb.Key("glue"), Timestamp: hlc.Timestamp{WallTime: 89999}}},
{key: MVCCKey{Key: roachpb.Key("foo"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45}}},
{key: MVCCKey{Key: roachpb.Key("bar"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45, Synthetic: true}}},
}
for _, test := range testCases {
t.Run("", func(t *testing.T) {
Expand Down Expand Up @@ -130,6 +130,32 @@ func TestMVCCAndEngineKeyEncodeDecode(t *testing.T) {
}
}

// TestMVCCAndEngineKeyDecodeSyntheticTimestamp tests decoding an MVCC key with
// a synthetic timestamp. The synthetic timestamp bit is now ignored during key
// encoding, but synthetic timestamps may still be present in the wild, so they
// must be decoded.
func TestMVCCAndEngineKeyDecodeSyntheticTimestamp(t *testing.T) {
defer leaktest.AfterTest(t)()

key := MVCCKey{Key: roachpb.Key("bar"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45, Synthetic: true}}

// encodedStr was computed from key using a previous version of the code that
// that included synthetic timestamps in the MVCC key encoding.
encodedStr := "6261720000000000000000630000002d010e"
encoded, err := hex.DecodeString(encodedStr)
require.NoError(t, err)

// Decode to demonstrate that the synthetic timestamp can be decoded.
eKeyDecoded, ok := DecodeEngineKey(encoded)
require.True(t, ok)
require.False(t, eKeyDecoded.IsLockTableKey())
require.True(t, eKeyDecoded.IsMVCCKey())
require.NoError(t, eKeyDecoded.Validate())
keyDecoded, err := eKeyDecoded.ToMVCCKey()
require.NoError(t, err)
require.Equal(t, key, keyDecoded)
}

func TestEngineKeyValidate(t *testing.T) {
defer leaktest.AfterTest(t)()
uuid1 := uuid.Must(uuid.FromString("6ba7b810-9dad-11d1-80b4-00c04fd430c8"))
Expand All @@ -141,7 +167,6 @@ func TestEngineKeyValidate(t *testing.T) {
{key: MVCCKey{Key: roachpb.Key("a")}},
{key: MVCCKey{Key: roachpb.Key("glue"), Timestamp: hlc.Timestamp{WallTime: 89999}}},
{key: MVCCKey{Key: roachpb.Key("foo"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45}}},
{key: MVCCKey{Key: roachpb.Key("bar"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45, Synthetic: true}}},

// Valid LockTableKeys.
{
Expand Down
11 changes: 2 additions & 9 deletions pkg/storage/mvcc_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,8 @@ func EncodeMVCCTimestampToBuf(buf []byte, ts hlc.Timestamp) []byte {
// buffer must have the correct size, and the timestamp must not be empty.
func encodeMVCCTimestampToBuf(buf []byte, ts hlc.Timestamp) {
binary.BigEndian.PutUint64(buf, uint64(ts.WallTime))
if ts.Logical != 0 || ts.Synthetic {
if ts.Logical != 0 {
binary.BigEndian.PutUint32(buf[mvccEncodedTimeWallLen:], uint32(ts.Logical))
if ts.Synthetic {
buf[mvccEncodedTimeWallLen+mvccEncodedTimeLogicalLen] = 1
}
}
}

Expand All @@ -296,12 +293,8 @@ func encodedMVCCKeyLength(key MVCCKey) int {
keyLen := len(key.Key) + mvccEncodedTimeSentinelLen
if !key.Timestamp.IsEmpty() {
keyLen += mvccEncodedTimeWallLen + mvccEncodedTimeLengthLen
if key.Timestamp.Logical != 0 || key.Timestamp.Synthetic {
if key.Timestamp.Logical != 0 {
keyLen += mvccEncodedTimeLogicalLen
if key.Timestamp.Synthetic {
// TODO(nvanbenschoten): stop writing Synthetic timestamps in v23.1.
keyLen += mvccEncodedTimeSyntheticLen
}
}
}
return keyLen
Expand Down
48 changes: 26 additions & 22 deletions pkg/storage/mvcc_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ func TestMVCCKeyCompare(t *testing.T) {
b0 := MVCCKey{roachpb.Key("b"), hlc.Timestamp{Logical: 0}}
b1 := MVCCKey{roachpb.Key("b"), hlc.Timestamp{Logical: 1}}
b2 := MVCCKey{roachpb.Key("b"), hlc.Timestamp{Logical: 2}}
b2S := MVCCKey{roachpb.Key("b"), hlc.Timestamp{Logical: 2, Synthetic: true}}

testcases := map[string]struct {
a MVCCKey
Expand All @@ -90,7 +89,6 @@ func TestMVCCKeyCompare(t *testing.T) {
"empty time lt set": {b0, b1, -1}, // empty MVCC timestamps sort before non-empty
"set time gt empty": {b1, b0, 1}, // empty MVCC timestamps sort before non-empty
"key time precedence": {a1, b2, -1}, // a before b, but 2 before 1; key takes precedence
"synthetic equal": {b2, b2S, 0}, // synthetic bit does not affect ordering
}
for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
Expand Down Expand Up @@ -153,10 +151,6 @@ func (k randMVCCKey) Generate(r *rand.Rand, size int) reflect.Value {
k.Key = []byte([...]string{"a", "b", "c"}[r.Intn(3)])
k.Timestamp.WallTime = r.Int63n(5)
k.Timestamp.Logical = r.Int31n(5)
if !k.Timestamp.IsEmpty() {
// NB: the zero timestamp cannot be synthetic.
k.Timestamp.Synthetic = r.Intn(2) != 0
}
return reflect.ValueOf(k)
}

Expand All @@ -168,16 +162,12 @@ func TestEncodeDecodeMVCCKeyAndTimestampWithLength(t *testing.T) {
ts hlc.Timestamp
encoded string // hexadecimal
}{
"empty": {"", hlc.Timestamp{}, "00"},
"only key": {"foo", hlc.Timestamp{}, "666f6f00"},
"no key": {"", hlc.Timestamp{WallTime: 1643550788737652545}, "0016cf10bc0505574109"},
"walltime": {"foo", hlc.Timestamp{WallTime: 1643550788737652545}, "666f6f0016cf10bc0505574109"},
"logical": {"foo", hlc.Timestamp{Logical: 65535}, "666f6f0000000000000000000000ffff0d"},
"synthetic": {"foo", hlc.Timestamp{Synthetic: true}, "666f6f00000000000000000000000000010e"},
"walltime and logical": {"foo", hlc.Timestamp{WallTime: 1643550788737652545, Logical: 65535}, "666f6f0016cf10bc050557410000ffff0d"},
"walltime and synthetic": {"foo", hlc.Timestamp{WallTime: 1643550788737652545, Synthetic: true}, "666f6f0016cf10bc0505574100000000010e"},
"logical and synthetic": {"foo", hlc.Timestamp{Logical: 65535, Synthetic: true}, "666f6f0000000000000000000000ffff010e"},
"all": {"foo", hlc.Timestamp{WallTime: 1643550788737652545, Logical: 65535, Synthetic: true}, "666f6f0016cf10bc050557410000ffff010e"},
"empty": {"", hlc.Timestamp{}, "00"},
"only key": {"foo", hlc.Timestamp{}, "666f6f00"},
"no key": {"", hlc.Timestamp{WallTime: 1643550788737652545}, "0016cf10bc0505574109"},
"walltime": {"foo", hlc.Timestamp{WallTime: 1643550788737652545}, "666f6f0016cf10bc0505574109"},
"logical": {"foo", hlc.Timestamp{Logical: 65535}, "666f6f0000000000000000000000ffff0d"},
"walltime and logical": {"foo", hlc.Timestamp{WallTime: 1643550788737652545, Logical: 65535}, "666f6f0016cf10bc050557410000ffff0d"},
}

buf := []byte{}
Expand Down Expand Up @@ -268,6 +258,26 @@ func TestDecodeUnnormalizedMVCCKey(t *testing.T) {
// keys that only contain (at most) a walltime.
equalToNormal: false,
},
"synthetic": {
encoded: "666f6f00000000000000000000000000010e",
expected: MVCCKey{Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 0, Logical: 0, Synthetic: true}},
equalToNormal: true,
},
"walltime and synthetic": {
encoded: "666f6f0016cf10bc0505574100000000010e",
expected: MVCCKey{Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1643550788737652545, Logical: 0, Synthetic: true}},
equalToNormal: true,
},
"logical and synthetic": {
encoded: "666f6f0000000000000000000000ffff010e",
expected: MVCCKey{Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 0, Logical: 65535, Synthetic: true}},
equalToNormal: true,
},
"walltime, logical, and synthetic": {
encoded: "666f6f0016cf10bc050557410000ffff010e",
expected: MVCCKey{Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1643550788737652545, Logical: 65535, Synthetic: true}},
equalToNormal: true,
},
}
for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
Expand Down Expand Up @@ -345,7 +355,6 @@ func BenchmarkEncodeMVCCKey(b *testing.B) {
"empty": {},
"walltime": {WallTime: 1643550788737652545},
"walltime+logical": {WallTime: 1643550788737652545, Logical: 4096},
"all": {WallTime: 1643550788737652545, Logical: 4096, Synthetic: true},
}
buf := make([]byte, 0, 65536)
for keyDesc, key := range keys {
Expand Down Expand Up @@ -585,11 +594,6 @@ func TestMVCCRangeKeyEncodedSize(t *testing.T) {
"only end": {MVCCRangeKey{EndKey: roachpb.Key("foo")}, 5},
"only walltime": {MVCCRangeKey{Timestamp: hlc.Timestamp{WallTime: 1}}, 11},
"only logical": {MVCCRangeKey{Timestamp: hlc.Timestamp{Logical: 1}}, 15},
"all": {MVCCRangeKey{
StartKey: roachpb.Key("start"),
EndKey: roachpb.Key("end"),
Timestamp: hlc.Timestamp{WallTime: 1, Logical: 1, Synthetic: true},
}, 24},
}
for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
Expand Down
4 changes: 0 additions & 4 deletions pkg/storage/pebble_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,6 @@ func makeRandEncodedKeys() [][]byte {
// 20% of keys have a logical component.
k.Timestamp.Logical = rng.Int31n(4) + 1
}
if rng.Int31n(1000) == 0 && !k.Timestamp.IsEmpty() {
// 0.1% of keys have a synthetic component.
k.Timestamp.Synthetic = true
}
keys[i] = EncodeMVCCKey(k)
}
return keys
Expand Down
Loading

0 comments on commit f05520f

Please sign in to comment.