Skip to content

Commit 7c20ebb

Browse files
committed
rowblk: fix IsLowerBound to account for synthetic suffix
`Iter.firstUserKey` has the synthetic prefix prepended but the on-disk suffix is preserved; suffix replacement is only applied per-returned key via `maybeReplaceSuffix`. Comparing `firstUserKey` directly against the probe could return true even though the iterator's actual first key (after synthetic suffix replacement) sorts strictly less than the probe, violating the `blockiter.Data.IsLowerBound` contract. When a synthetic suffix is in effect, only compare the prefix portion of `firstUserKey` and require it to be strictly greater than the probe. The contract permits false negatives, so this conservative comparison is sound and avoids reasoning about the unknown suffix. Adds `TestIsLowerBoundRandomized` which cross-checks `IsLowerBound` against `First()` across random blocks and transform combinations, and documents the transform state of `firstUserKey`.
1 parent 8b6bc41 commit 7c20ebb

2 files changed

Lines changed: 89 additions & 0 deletions

File tree

sstable/rowblk/rowblk_iter.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,10 @@ type Iter struct {
187187
cachedBuf []byte
188188
handle block.BufferHandle
189189
// for block iteration for already loaded blocks.
190+
//
191+
// firstUserKey is the user key of the first KV in the block. It has the
192+
// synthetic prefix applied (if any), but does NOT have the synthetic suffix
193+
// applied — the on-disk suffix is preserved.
190194
firstUserKey []byte
191195
lazyValueHandling struct {
192196
getValue block.GetLazyValueForPrefixAndValueHandler
@@ -541,6 +545,15 @@ func (i *Iter) cacheEntry() {
541545
// IsLowerBound implements the block.DataBlockIterator interface.
542546
func (i *Iter) IsLowerBound(k []byte) bool {
543547
// Note: we ignore HideObsoletePoints, but false negatives are allowed.
548+
if i.transforms.HasSyntheticSuffix() {
549+
// firstUserKey has the synthetic prefix applied, but its suffix is the
550+
// original on-disk suffix — not the synthetic suffix that the iterator will
551+
// actually return. Instead of applying the synthetic suffix, we
552+
// conservatively require firstUserKey's prefix portion to be strictly
553+
// greater than k (false negatives are allowed).
554+
firstPrefix := i.firstUserKey[:i.split(i.firstUserKey)]
555+
return i.cmp(firstPrefix, k) > 0
556+
}
544557
return i.cmp(i.firstUserKey, k) >= 0
545558
}
546559

sstable/rowblk/rowblk_iter_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@ import (
88
"bytes"
99
"fmt"
1010
"math"
11+
"math/rand/v2"
1112
"os"
1213
"runtime"
14+
"slices"
1315
"strconv"
1416
"strings"
1517
"testing"
18+
"time"
1619
"unsafe"
1720

1821
"github.com/cockroachdb/datadriven"
@@ -631,3 +634,76 @@ func TestMultipleKVBlockRestartsOverflow(t *testing.T) {
631634
func ikey(s string) base.InternalKey {
632635
return base.InternalKey{UserKey: []byte(s)}
633636
}
637+
638+
func TestIsLowerBoundRand(t *testing.T) {
639+
cmp := testkeys.Comparer.Compare
640+
suffixCmp := testkeys.Comparer.ComparePointSuffixes
641+
split := testkeys.Comparer.Split
642+
643+
prefixes := []string{"a", "b", "c", "d", "e", "p/", "zzz/"}
644+
suffixes := []string{"", "@1", "@5", "@10", "@20", "@100"}
645+
646+
seed := uint64(time.Now().UnixNano())
647+
t.Logf("seed: %d", seed)
648+
rng := rand.New(rand.NewPCG(seed, seed))
649+
randKey := func() string {
650+
return prefixes[rng.IntN(len(prefixes))] + suffixes[rng.IntN(len(suffixes))]
651+
}
652+
653+
for iter := 0; iter < 1000; iter++ {
654+
// Construct a block with a random combination of prefixes and suffixes.
655+
n := 1 + rng.IntN(20)
656+
keySet := make(map[string]struct{}, n)
657+
for len(keySet) < n {
658+
keySet[randKey()] = struct{}{}
659+
}
660+
keys := make([]string, 0, n)
661+
for k := range keySet {
662+
keys = append(keys, k)
663+
}
664+
slices.SortFunc(keys, func(a, b string) int { return cmp([]byte(a), []byte(b)) })
665+
666+
w := &Writer{RestartInterval: 1 + rng.IntN(4)}
667+
for _, k := range keys {
668+
require.NoError(t, w.Add(ikey(k), nil))
669+
}
670+
blk := w.Finish()
671+
672+
// Pick a random prefix and suffix for the transform.
673+
var synthPrefix, synthSuffix []byte
674+
if rand.IntN(2) == 0 {
675+
synthPrefix = []byte(prefixes[rng.IntN(len(prefixes))])
676+
}
677+
if rand.IntN(2) == 0 {
678+
synthSuffix = []byte(suffixes[rng.IntN(len(suffixes))])
679+
}
680+
transforms := block.IterTransforms{
681+
SyntheticPrefixAndSuffix: block.MakeSyntheticPrefixAndSuffix(synthPrefix, synthSuffix),
682+
}
683+
684+
it, err := NewIter(cmp, suffixCmp, split, blk, transforms)
685+
require.NoError(t, err)
686+
687+
kv := it.First()
688+
require.NotNil(t, kv)
689+
// firstKey is the key after the transform.
690+
firstKey := slices.Clone(kv.K.UserKey)
691+
692+
for probe := 0; probe < 50; probe++ {
693+
probeKey := []byte(randKey())
694+
if rand.IntN(2) == 0 {
695+
// The transformed block keys are random keys with synthPrefix
696+
// prepended. To generate more interesting probe keys, sometimes prepend
697+
// this prefix to the random key.
698+
probeKey = append(slices.Clip(synthPrefix), probeKey...)
699+
}
700+
if it.IsLowerBound(probeKey) {
701+
if cmp(firstKey, probeKey) < 0 {
702+
t.Fatalf("IsLowerBound(%q)=true but First()=%q < probe (transforms=%+v, keys=%v)",
703+
probeKey, firstKey, transforms, keys)
704+
}
705+
}
706+
}
707+
require.NoError(t, it.Close())
708+
}
709+
}

0 commit comments

Comments
 (0)