Skip to content

Commit

Permalink
kv: include replicated locks in replica consistency checks
Browse files Browse the repository at this point in the history
Fixes cockroachdb#111294.

This commit adds handling of replicated locks in replicate consistency
checks. They are iterated over as part of MVCCStats calculations and
hashed similarly to point keys.

The commit also ensures that the iteration is properly throttled.

Release note: None
  • Loading branch information
nvanbenschoten authored and Thomas Hardy committed Oct 4, 2023
1 parent 7b832be commit 5116b78
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 3 deletions.
1 change: 1 addition & 0 deletions pkg/kv/kvserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ go_test(
"//pkg/util/timeutil",
"//pkg/util/tracing",
"//pkg/util/tracing/tracingpb",
"//pkg/util/uint128",
"//pkg/util/uuid",
"@com_github_cockroachdb_cockroach_go_v2//crdb",
"@com_github_cockroachdb_datadriven//:datadriven",
Expand Down
45 changes: 42 additions & 3 deletions pkg/kv/kvserver/replica_consistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/rditer"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
Expand Down Expand Up @@ -485,6 +486,7 @@ func CalcReplicaDigest(
var intBuf [8]byte
var legacyTimestamp hlc.LegacyTimestamp
var timestampBuf []byte
var uuidBuf [uuid.Size]byte
hasher := sha512.New()

if efos, ok := snap.(storage.EventuallyFileOnlyReader); ok {
Expand Down Expand Up @@ -532,6 +534,7 @@ func CalcReplicaDigest(
if _, err := hasher.Write(intBuf[:]); err != nil {
return err
}
// Encode the key.
if _, err := hasher.Write(unsafeKey.Key); err != nil {
return err
}
Expand All @@ -547,6 +550,7 @@ func CalcReplicaDigest(
if _, err := hasher.Write(timestampBuf); err != nil {
return err
}
// Encode the value.
_, err := hasher.Write(unsafeValue)
return err
}
Expand All @@ -571,6 +575,7 @@ func CalcReplicaDigest(
if _, err := hasher.Write(intBuf[:]); err != nil {
return err
}
// Encode the key.
if _, err := hasher.Write(rangeKV.RangeKey.StartKey); err != nil {
return err
}
Expand All @@ -589,14 +594,48 @@ func CalcReplicaDigest(
if _, err := hasher.Write(timestampBuf); err != nil {
return err
}
// Encode the value.
_, err = hasher.Write(rangeKV.Value)
return err
}

visitors.LockTableKey = func(unsafeKey storage.LockTableKey, unsafeValue []byte) error {
// TODO(nvanbenschoten): rate limit scan through lock table and add to
// checksum to be included in consistency checks.
return nil
// Assert that the lock is not an intent. Intents are handled by the
// PointKey visitor function, not by the LockTableKey visitor function.
if unsafeKey.Strength == lock.Intent {
return errors.AssertionFailedf("unexpected intent lock in LockTableKey visitor: %s", unsafeKey)
}
// Rate limit the scan through the lock table.
if err := wait(int64(len(unsafeKey.Key) + len(unsafeValue))); err != nil {
return err
}
// Encode the length of the key and value.
binary.LittleEndian.PutUint64(intBuf[:], uint64(len(unsafeKey.Key)))
if _, err := hasher.Write(intBuf[:]); err != nil {
return err
}
binary.LittleEndian.PutUint64(intBuf[:], uint64(len(unsafeValue)))
if _, err := hasher.Write(intBuf[:]); err != nil {
return err
}
// Encode the key.
if _, err := hasher.Write(unsafeKey.Key); err != nil {
return err
}
// NOTE: this is not the same strength encoding that the actual lock
// table version uses. For that, see getByteForReplicatedLockStrength.
strengthBuf := intBuf[:1]
strengthBuf[0] = byte(unsafeKey.Strength)
if _, err := hasher.Write(strengthBuf); err != nil {
return err
}
copy(uuidBuf[:], unsafeKey.TxnUUID.GetBytes())
if _, err := hasher.Write(uuidBuf[:]); err != nil {
return err
}
// Encode the value.
_, err := hasher.Write(unsafeValue)
return err
}

// In statsOnly mode, we hash only the RangeAppliedState. In regular mode, hash
Expand Down
24 changes: 24 additions & 0 deletions pkg/kv/kvserver/replica_consistency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import (

"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
"github.com/cockroachdb/cockroach/pkg/testutils/echotest"
Expand All @@ -31,6 +33,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/quotapool"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/cockroach/pkg/util/uint128"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/stretchr/testify/require"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -315,6 +318,27 @@ func TestReplicaChecksumSHA512(t *testing.T) {
fmt.Fprintf(sb, "checksum%d: %x\n", i+1, rd.SHA512)
}

// We then do the same for replicated locks.
locks := []struct {
key string
str lock.Strength
txnID int64
}{
{"a", lock.Exclusive, 1},
{"b", lock.Shared, 1},
{"b", lock.Shared, 2},
}

for i, l := range locks {
txnID := uuid.FromUint128(uint128.FromInts(0, uint64(l.txnID)))
txn := &roachpb.Transaction{TxnMeta: enginepb.TxnMeta{ID: txnID}}
require.NoError(t, storage.MVCCAcquireLock(ctx, eng, txn, l.str, roachpb.Key(l.key), nil, 0))

rd, err = CalcReplicaDigest(ctx, desc, eng, kvpb.ChecksumMode_CHECK_FULL, unlim)
require.NoError(t, err)
fmt.Fprintf(sb, "checksum%d: %x\n", i+1, rd.SHA512)
}

// Run another check to obtain stats for the final state.
rd, err = CalcReplicaDigest(ctx, desc, eng, kvpb.ChecksumMode_CHECK_FULL, unlim)
require.NoError(t, err)
Expand Down
5 changes: 5 additions & 0 deletions pkg/kv/kvserver/testdata/replica_consistency_sha512
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@ checksum3: 05186bceae59a178713407959a26110715a1e299e6a9f1b37fc3e0f8d5a0c66bedbff
checksum4: 4f5cc8176d559bfab8e52b74851b103fd73b9e713ce12aa380a16fe177ca6e21db75e3e85a58341ab437a5a766a071a2fe6e1f03841d334da7be2295794eb813
checksum5: 3c5d5856a626aa29913e9790033b9c23b6dc5e42bdf2e665f7b60f58bec495adc246bf4e5f5bf1acbfc78c713f2ec7820b4ba7202897bb9f824a0b7b9e9cc98d
checksum6: ebe7fd3f41a68c2608a8b10dcc9db3b39bdb6c097d3fd99411e89d75419bb58dd80faf9846aa5e47d8cabc9dcfc894c6ea58f7e035eaaa3ee55c31faed2c8000
checksum1: 83ae68fc9e2b083f5991c0a74bdaa712f3e794dc78b0bcf3467d8a7f7bba41c933f9834a05a98136a92dbabf1ea5662fa618fdadd9f8478357d28be0a53714e8
checksum2: 066a27ad115e2db378ef62071b0f1d1ebc01b43acdf7a52a596cc94ee243b2077fe83ffb9645bfb76c0bd34d3cfe884387ffe4aa9642d22e27c4cace4cce2ce9
checksum3: 49bb7da05ce4ae4a4f46037be263f99b2b2c25237c9f0d7af65f992f50c3adba7fe2dd8493bfab107d709b69119671357aeecfb065782ef3a84ea227d3f05cb6
stats: {
"liveBytes": "53",
"liveCount": "2",
"keyBytes": "42",
"keyCount": "3",
"valBytes": "44",
"valCount": "4",
"lockBytes": "168",
"lockCount": "3",
"rangeKeyCount": "2",
"rangeKeyBytes": "26",
"rangeValCount": "2",
Expand Down

0 comments on commit 5116b78

Please sign in to comment.