You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit bf238af
Browse filesBrowse the repository at this point in the historyBrowse files
Summary:
Why the change is required:
The crash test found prefix_same_as_start iterators returning wrong keys in
two ways, both caused by child iterators using prefix filters (SST prefix
filter, memtable prefix bloom, LevelIterator prefix exhaustion) for a key in a
different prefix than the one the caller iterates:
1. SeekToFirst() without iterate_lower_bound and SeekToLast() without
iterate_upper_bound have no seek key to take the prefix from. With a range
tombstone that crosses a prefix boundary, MergingIterator reseeks older
children to the tombstone boundary key, a prefix filter can drop a child,
and the scan can return a wrong first key. Read-path tombstone conversion
(min_tombstones_for_range_conversion, EXPERIMENTAL) trusts these scans, so
a skipped live key could be covered by a converted tombstone and deleted
for good.
2. With prefix_same_as_start and total_order_seek both set, a direction change
(Next() after Prev()) can skip keys inside the seek prefix. Under
DBOptions::prefix_seek_opt_in_only, total_order_seek is forced on, so every
prefix_same_as_start iterator is exposed.
Previous behavior:
1. Unbounded SeekToFirst()/SeekToLast() in prefix seek mode were never
promised to match total order (the option docs call keys outside the seek
prefix undefined, and the Prefix-Seek wiki says these calls are not well
supported), but they ran and could return wrong results.
2. Since prefix_seek_opt_in_only was added (9.7.0), prefix_same_as_start and
auto_prefix_mode enable prefix filtering even with total_order_seek=true,
so for iterators total_order_seek only turns off the implicit prefix seek
of default ReadOptions. This was stated in that release note, but not in
options.h, where total_order_seek still read as "always total order". Each
iterator layer read total_order_seek on its own, and DBIter still
treated total_order_seek=true as "the inner iterator is total order". So
DBIter::ReverseToForward() skipped its reseek to the current key, and
MergingIterator::SwitchToForward() reseeked the other children to the
previous key, which can have another prefix. A child without that prefix
was dropped by its prefix filter, losing its keys in the seek prefix.
New behavior:
1. With DBOptions::prefix_seek_opt_in_only, a prefix_same_as_start iterator
leaves itself !Valid() with Status::NotSupported on SeekToFirst() without
iterate_lower_bound and SeekToLast() without iterate_upper_bound. The
bounded forms still become Seek()/SeekForPrev() and work. Without
prefix_seek_opt_in_only, iteration results of these calls are unchanged,
but a scan they start under prefix_same_as_start no longer creates
read-path range tombstones, until the next Seek()/SeekForPrev().
2. prefix_same_as_start and auto_prefix_mode take precedence over
total_order_seek in every iterator layer. A prefix_same_as_start iterator
behaves the same with or without total_order_seek, and so with or without
prefix_seek_opt_in_only for Seek()-started scans.
Total order and auto_prefix_mode iteration results are not affected.
Implementation:
- GetIteratorPrefixMode() in options_helper.h decides once how an iterator
uses the prefix extractor (kTotalOrder, kAuto, kPrefix), with the order
prefix_same_as_start > auto_prefix_mode > total_order_seek > implicit prefix
seek. DBIter, the MergingIterator prefix flag, LevelIterator, MemTable, the
block-based table filter and hash index, and PlainTable use it instead of
reading total_order_seek themselves. Only DBIter's total-order assumption is
a correctness fix; for the others the result is unchanged or the
prefix_same_as_start + total_order_seek combination gets the same prefix
optimizations as plain prefix_same_as_start.
- DBIter returns NotSupported from unbounded SeekToFirst()/SeekToLast() for
prefix_same_as_start under prefix_seek_opt_in_only. It also remembers when
the current scan was started by an unbounded SeekToFirst()/SeekToLast()
under prefix_same_as_start, and MaybeInsertRangeTombstone(), the only place
that writes a converted tombstone, does nothing then.
- options.h: ReadOptions::total_order_seek now starts with its iterator
meaning (turns off the implicit prefix seek; prefix_same_as_start and
auto_prefix_mode take precedence; forced on by prefix_seek_opt_in_only),
auto_prefix_mode notes that prefix_same_as_start takes precedence, and
prefix_seek_opt_in_only notes that prefix_same_as_start iterators still use
prefix seek. prefix_seek_opt_in_only and prefix_same_as_start document the
unbounded SeekToFirst()/SeekToLast() behavior.
- Release notes: a behavior change for the NotSupported and conversion change,
and a bug fix for the direction change, which also states that
prefix_same_as_start now consistently takes precedence over total_order_seek
and auto_prefix_mode.
- db_stress: new prefix_seek_opt_in_only flag, randomized by db_crashtest.py.
TestIterate sometimes sets total_order_seek together with
prefix_same_as_start, and VerifyIterator uses GetIteratorPrefixMode(). Under
prefix_seek_opt_in_only it checks that unbounded SeekToFirst()/SeekToLast()
return NotSupported. Otherwise the prefix_same_as_start SeekToFirst() check
only runs when no range tombstone can exist (no DeleteRange, no standalone
range deletion ingestion, no read-path conversion), since it would keep
hitting the known limitation.
Safety, compatibility, and limitations:
- No API or format change. Users with prefix_seek_opt_in_only who call
unbounded SeekToFirst()/SeekToLast() on a prefix_same_as_start iterator now
get NotSupported; they can set a bound, or use total order or
auto_prefix_mode.
- prefix_same_as_start with total_order_seek or prefix_seek_opt_in_only now
reseeks on a direction change, the cost plain prefix_same_as_start already
pays. prefix_same_as_start with auto_prefix_mode now uses the SST prefix
filter for the seek prefix, as the memtable already did.
- Without prefix_seek_opt_in_only, unbounded SeekToFirst()/SeekToLast() under
prefix seek can still skip live keys when range tombstones cross prefix
boundaries, and unbounded legacy prefix-mode scans have other known holes.
These stay documented limitations.
Differential Revision: D129901533
0 commit comments