Skip to content

Commit fde0deb

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 84f4838 commit fde0deb

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
@@ -188,6 +188,10 @@ type Iter struct {
188188
cachedBuf []byte
189189
handle block.BufferHandle
190190
// for block iteration for already loaded blocks.
191+
//
192+
// firstUserKey is the user key of the first KV in the block. It has the
193+
// synthetic prefix applied (if any), but does NOT have the synthetic suffix
194+
// applied — the on-disk suffix is preserved.
191195
firstUserKey []byte
192196
lazyValueHandling struct {
193197
getValue block.GetInternalValueForPrefixAndValueHandler
@@ -541,6 +545,15 @@ func (i *Iter) cacheEntry() {
541545
// IsLowerBound implements the blockiter.Data 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,8 +8,11 @@ import (
88
"bytes"
99
"context"
1010
"fmt"
11+
"math/rand/v2"
12+
"slices"
1113
"strings"
1214
"testing"
15+
"time"
1316
"unsafe"
1417

1518
"github.com/cockroachdb/datadriven"
@@ -482,3 +485,76 @@ func TestBlockSyntheticSuffix(t *testing.T) {
482485
func ikey(s string) base.InternalKey {
483486
return base.InternalKey{UserKey: []byte(s)}
484487
}
488+
489+
func TestIsLowerBoundRand(t *testing.T) {
490+
cmp := testkeys.Comparer.Compare
491+
suffixCmp := testkeys.Comparer.ComparePointSuffixes
492+
split := testkeys.Comparer.Split
493+
494+
prefixes := []string{"a", "b", "c", "d", "e", "p/", "zzz/"}
495+
suffixes := []string{"", "@1", "@5", "@10", "@20", "@100"}
496+
497+
seed := uint64(time.Now().UnixNano())
498+
t.Logf("seed: %d", seed)
499+
rng := rand.New(rand.NewPCG(seed, seed))
500+
randKey := func() string {
501+
return prefixes[rng.IntN(len(prefixes))] + suffixes[rng.IntN(len(suffixes))]
502+
}
503+
504+
for iter := 0; iter < 1000; iter++ {
505+
// Construct a block with a random combination of prefixes and suffixes.
506+
n := 1 + rng.IntN(20)
507+
keySet := make(map[string]struct{}, n)
508+
for len(keySet) < n {
509+
keySet[randKey()] = struct{}{}
510+
}
511+
keys := make([]string, 0, n)
512+
for k := range keySet {
513+
keys = append(keys, k)
514+
}
515+
slices.SortFunc(keys, func(a, b string) int { return cmp([]byte(a), []byte(b)) })
516+
517+
w := &Writer{RestartInterval: 1 + rng.IntN(4)}
518+
for _, k := range keys {
519+
require.NoError(t, w.Add(ikey(k), nil))
520+
}
521+
blk := w.Finish()
522+
523+
// Pick a random prefix and suffix for the transform.
524+
var synthPrefix, synthSuffix []byte
525+
if rand.IntN(2) == 0 {
526+
synthPrefix = []byte(prefixes[rng.IntN(len(prefixes))])
527+
}
528+
if rand.IntN(2) == 0 {
529+
synthSuffix = []byte(suffixes[rng.IntN(len(suffixes))])
530+
}
531+
transforms := blockiter.Transforms{
532+
SyntheticPrefixAndSuffix: blockiter.MakeSyntheticPrefixAndSuffix(synthPrefix, synthSuffix),
533+
}
534+
535+
it, err := NewIter(cmp, suffixCmp, split, blk, transforms)
536+
require.NoError(t, err)
537+
538+
kv := it.First()
539+
require.NotNil(t, kv)
540+
// firstKey is the key after the transform.
541+
firstKey := slices.Clone(kv.K.UserKey)
542+
543+
for probe := 0; probe < 50; probe++ {
544+
probeKey := []byte(randKey())
545+
if rand.IntN(2) == 0 {
546+
// The transformed block keys are random keys with synthPrefix
547+
// prepended. To generate more interesting probe keys, sometimes prepend
548+
// this prefix to the random key.
549+
probeKey = append(slices.Clip(synthPrefix), probeKey...)
550+
}
551+
if it.IsLowerBound(probeKey) {
552+
if cmp(firstKey, probeKey) < 0 {
553+
t.Fatalf("IsLowerBound(%q)=true but First()=%q < probe (transforms=%+v, keys=%v)",
554+
probeKey, firstKey, transforms, keys)
555+
}
556+
}
557+
}
558+
require.NoError(t, it.Close())
559+
}
560+
}

0 commit comments

Comments
 (0)