Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: use concrete pebbleIterator in verifyingIterator #86478

Merged
merged 2 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions pkg/storage/bench_pebble_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,9 +494,13 @@ func BenchmarkCheckSSTConflicts(b *testing.B) {
func BenchmarkSSTIterator(b *testing.B) {
for _, numKeys := range []int{1, 100, 10000} {
b.Run(fmt.Sprintf("keys=%d", numKeys), func(b *testing.B) {
for _, verify := range []bool{false, true} {
b.Run(fmt.Sprintf("verify=%t", verify), func(b *testing.B) {
runSSTIterator(b, numKeys, verify)
for _, variant := range []string{"legacy", "pebble"} {
b.Run(fmt.Sprintf("variant=%s", variant), func(b *testing.B) {
for _, verify := range []bool{false, true} {
b.Run(fmt.Sprintf("verify=%t", verify), func(b *testing.B) {
runSSTIterator(b, variant, numKeys, verify)
})
}
})
}
})
Expand Down
22 changes: 20 additions & 2 deletions pkg/storage/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1728,7 +1728,7 @@ func runCheckSSTConflicts(
}
}

func runSSTIterator(b *testing.B, numKeys int, verify bool) {
func runSSTIterator(b *testing.B, variant string, numKeys int, verify bool) {
keyBuf := append(make([]byte, 0, 64), []byte("key-")...)
value := MVCCValue{Value: roachpb.MakeValueFromBytes(bytes.Repeat([]byte("a"), 128))}

Expand All @@ -1744,9 +1744,27 @@ func runSSTIterator(b *testing.B, numKeys int, verify bool) {
}
sstWriter.Close()

var makeSSTIterator func(data []byte, verify bool) (SimpleMVCCIterator, error)
switch variant {
case "legacy":
makeSSTIterator = func(data []byte, verify bool) (SimpleMVCCIterator, error) {
return NewMemSSTIterator(data, verify)
}
case "pebble":
makeSSTIterator = func(data []byte, verify bool) (SimpleMVCCIterator, error) {
return NewPebbleMemSSTIterator(data, verify, IterOptions{
KeyTypes: IterKeyTypePointsAndRanges,
LowerBound: keys.MinKey,
UpperBound: keys.MaxKey,
})
}
default:
b.Fatalf("unknown variant %q", variant)
}

b.ResetTimer()
for i := 0; i < b.N; i++ {
iter, err := NewMemSSTIterator(sstFile.Bytes(), verify)
iter, err := makeSSTIterator(sstFile.Bytes(), verify)
if err != nil {
b.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/sst_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewPebbleMultiMemSSTIterator(
return nil, err
}
if verify {
iter = NewVerifyingMVCCIterator(iter)
iter = newVerifyingMVCCIterator(iter.(*pebbleIterator))
}
return iter, nil
}
Expand Down
71 changes: 37 additions & 34 deletions pkg/storage/verifying_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,34 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/uuid"
)

// VerifyingMVCCIterator is an MVCC iterator that wraps an arbitrary MVCC
// iterator and verifies roachpb.Value checksums for encountered values.
type VerifyingMVCCIterator struct {
MVCCIterator

valid bool
err error
key MVCCKey
value []byte
// verifyingMVCCIterator is an MVCC iterator that wraps a pebbleIterator and
// verifies roachpb.Value checksums for encountered values.
type verifyingMVCCIterator struct {
*pebbleIterator // concrete type to avoid dynamic dispatch

valid bool
err error
key MVCCKey
value []byte
hasPoint bool
hasRange bool
}

// NewVerifyingMVCCIterator creates a new VerifyingMVCCIterator.
func NewVerifyingMVCCIterator(iter MVCCIterator) MVCCIterator {
return &VerifyingMVCCIterator{MVCCIterator: iter}
// newVerifyingMVCCIterator creates a new VerifyingMVCCIterator.
func newVerifyingMVCCIterator(iter *pebbleIterator) MVCCIterator {
return &verifyingMVCCIterator{pebbleIterator: iter}
}

// saveAndVerify fetches the current key and value, saves them in the iterator,
// and verifies the value.
func (i *VerifyingMVCCIterator) saveAndVerify() {
if i.valid, i.err = i.MVCCIterator.Valid(); !i.valid || i.err != nil {
func (i *verifyingMVCCIterator) saveAndVerify() {
if i.valid, i.err = i.pebbleIterator.Valid(); !i.valid || i.err != nil {
return
}
i.key = i.MVCCIterator.UnsafeKey()
if hasPoint, _ := i.MVCCIterator.HasPointAndRange(); hasPoint {
i.value = i.MVCCIterator.UnsafeValue()
i.key = i.pebbleIterator.UnsafeKey()
i.hasPoint, i.hasRange = i.pebbleIterator.HasPointAndRange()
if i.hasPoint {
i.value = i.pebbleIterator.UnsafeValue()
if i.key.IsValue() {
mvccValue, ok, err := tryDecodeSimpleMVCCValue(i.value)
if !ok && err == nil {
Expand All @@ -58,57 +61,57 @@ func (i *VerifyingMVCCIterator) saveAndVerify() {
}

// Next implements MVCCIterator.
func (i *VerifyingMVCCIterator) Next() {
i.MVCCIterator.Next()
func (i *verifyingMVCCIterator) Next() {
i.pebbleIterator.Next()
i.saveAndVerify()
}

// NextKey implements MVCCIterator.
func (i *VerifyingMVCCIterator) NextKey() {
i.MVCCIterator.NextKey()
func (i *verifyingMVCCIterator) NextKey() {
i.pebbleIterator.NextKey()
i.saveAndVerify()
}

// Prev implements MVCCIterator.
func (i *VerifyingMVCCIterator) Prev() {
i.MVCCIterator.Prev()
func (i *verifyingMVCCIterator) Prev() {
i.pebbleIterator.Prev()
i.saveAndVerify()
}

// SeekGE implements MVCCIterator.
func (i *VerifyingMVCCIterator) SeekGE(key MVCCKey) {
i.MVCCIterator.SeekGE(key)
func (i *verifyingMVCCIterator) SeekGE(key MVCCKey) {
i.pebbleIterator.SeekGE(key)
i.saveAndVerify()
}

// SeekIntentGE implements MVCCIterator.
func (i *VerifyingMVCCIterator) SeekIntentGE(key roachpb.Key, txnUUID uuid.UUID) {
i.MVCCIterator.SeekIntentGE(key, txnUUID)
func (i *verifyingMVCCIterator) SeekIntentGE(key roachpb.Key, txnUUID uuid.UUID) {
i.pebbleIterator.SeekIntentGE(key, txnUUID)
i.saveAndVerify()
}

// SeekLT implements MVCCIterator.
func (i *VerifyingMVCCIterator) SeekLT(key MVCCKey) {
i.MVCCIterator.SeekLT(key)
func (i *verifyingMVCCIterator) SeekLT(key MVCCKey) {
i.pebbleIterator.SeekLT(key)
i.saveAndVerify()
}

// UnsafeKey implements MVCCIterator.
func (i *VerifyingMVCCIterator) UnsafeKey() MVCCKey {
func (i *verifyingMVCCIterator) UnsafeKey() MVCCKey {
return i.key
}

// UnsafeValue implements MVCCIterator.
func (i *VerifyingMVCCIterator) UnsafeValue() []byte {
func (i *verifyingMVCCIterator) UnsafeValue() []byte {
return i.value
}

// Valid implements MVCCIterator.
func (i *VerifyingMVCCIterator) Valid() (bool, error) {
func (i *verifyingMVCCIterator) Valid() (bool, error) {
return i.valid, i.err
}

// HasPointAndRange implements MVCCIterator.
func (i *VerifyingMVCCIterator) HasPointAndRange() (bool, bool) {
return i.MVCCIterator.HasPointAndRange()
func (i *verifyingMVCCIterator) HasPointAndRange() (bool, bool) {
return i.hasPoint, i.hasRange
}