Skip to content

sstable: check bloom filter before generating synthetic key #5638

@jbowens

Description

@jbowens

Currently the SeekPrefixGE lazy positioning implementation added in ca74226 generates the synthetic key before checking the bloom filter:

// If there's a maximum suffix property configured and the seek key contains
// a suffix (len(key) > len(prefix)), we might be able to defer actually
// performing the seek and potentially loading additional blocks.
// However, we can only perform the synthetic key optimization if the prefix is
// wholly contained within the sstable's bounds. If the sought prefix is NOT wholly
// contained within the sstable, it might be split across multiple sstables. If the
// first sstable with bounds that overlap the prefix does not actually contain a point
// key with the prefix, the sstable's properties' max suffix doesn't reflect the max of the prefix.
// And so in such cases, performing the synthetic key optimization might return a wrong kv to the caller.
if i.maximumSuffixProperty != nil && len(key) > len(prefix) && i.readEnv.InternalBounds != nil {
prop := i.reader.UserProperties[i.maximumSuffixProperty.Name()]
var maxSuffix []byte
var ok bool
var err error
if prop != "" {
// Check if the synthetic suffix is present
if i.transforms.HasSyntheticSuffix() {
if maxSuffix = i.transforms.SyntheticSuffix(); maxSuffix != nil {
ok = true
}
} else {
maxSuffix, ok, err = i.maximumSuffixProperty.Extract(i.synthetic.maxSuffixBuf[:0],
unsafe.Slice(unsafe.StringData(prop), len(prop)))
if err != nil {
i.err = err
return nil
} else if ok {
i.synthetic.maxSuffixBuf = maxSuffix
}
}
}
// We have a max suffix. If the seek key's suffix is less than the
// table's max suffix, return a synthetic key with that max suffix.
// We'll only actually perform the seek if the synthetic key rises to
// the top of the iterator's heap, and the iterator is Nexted.
// During a RangeDelIter, the prefix passed and the prefix in the key might not be the same.
if ok && maxSuffix != nil && i.reader.Comparer.ComparePointSuffixes(key[i.reader.Comparer.Split(key):], maxSuffix) < 0 {
smallest := i.readEnv.InternalBounds.SmallestUserKey()
smallest = i.reader.Comparer.Split.Prefix(smallest)
largest := i.readEnv.InternalBounds.LargestUserKey()
largest = i.reader.Comparer.Split.Prefix(largest)
if i.cmp(prefix, smallest) > 0 && i.cmp(prefix, largest) < 0 {
// Build the synthetic key.
i.synthetic.kv.K.UserKey = append(append(i.synthetic.kv.K.UserKey[:0], prefix...), maxSuffix...)
i.synthetic.kv.K.Trailer = base.MakeTrailer(base.SeqNumMax, base.InternalKeyKindSyntheticKey)
i.synthetic.kv.V = base.InternalValue{}
i.synthetic.atSyntheticKey = true
// TODO(jackson): I think this copy of the seek key is necessary,
// but we should confirm and document exactly why--I think the seek
// key may be from a range tombstone iterator that is not guaranteed
// to still be open by the time singleLevelIterator.Next is called
// and we use the seek key to actually perform the seek.
i.synthetic.seekKey = append(i.synthetic.seekKey[:0], key...)
return &i.synthetic.kv
}
}
}

We should reverse the order, so that we generate fewer synthetic keys and need to perform fewer key comparisons within the heap.

Jira issue: PEBBLE-1297

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions