Skip to content

Conversation

@pdillinger
Copy link
Contributor

@pdillinger pdillinger commented May 9, 2023

Summary: Work In Progress / Request For Comments

Introduces a new struct SliceBound that provides a general way of specifying upper
and lower bounds for ranges of keys. Eventually this could be used for many APIs, including
DeleteRange, CompactRange, Iterator bounds, Iterator Seek locations, and more. This is intended
to solve many related problems:

  • Some APIs use exclusive upper bounds and some use inclusive upper bounds. This is confusing
    and can easily cause bugs. It is more user-friendly if exclusivity follows from the specification of
    the bound itself, not what this particular API prefers.
  • Some bounds are not constructible as keys. For example, if I'm using the reverse bytewise
    comparator and want to seek to the start of prefix "foo", the best I can do is seek to "foo"
    followed by a more 0xff chars than I hope to actually use, or seek to "fop" (nullifying opportunity
    for prefix seek optimization) and skip over any "fop" entry. See also issue No way to specify infinite upper bound to query properties/approximate size #11027
  • It appears most APIs expect bounds to include user timestamps (when enabled). This is not
    really documented and could easily be a source of bugs. Like exclusivity, it's much more
    user-friendly if the bound itself specifies whether it includes a user timestamp, rather than
    committing each API as interpreting one way or another. (By the way, the existing interpretation
    of bounds seems counter to the idea that the layout of user timestamps in keys is hidden
    from the user, except in the implementation of Comparator.)
  • We currently assume certain values for min and max user timestamp, which limits
    customization of the feature (at least in theory). SliceBound should offer a way to name
    a position before all or after all versions of a key, without assuming a meaning for certain
    timestamp values.
  • We have some fragile code dealing with the range of keys included in an SST file. The range
    it typically inclusive, but can be exclusive if the last key is only referring to the upper bound of
    a range tombstone. If we translate these ranges to SliceBound, they fit into a generic framework
    for key ranges, including checking overlap, etc.
  • auto_prefix_mode is a rather awkward and somewhat broken way to prefix filtering, often
    requiring copying & modifying keys for bounds. prefix_same_as_start is arguably bug-prone
    because the meaning of a read operation is tied to the current prefix extractor. SliceBound
    should be able to solve these issues by making it simple to scan a specific prefix, regardless
    of current prefix extractor setting.

There is likely to be a performance cost associated with comparing against these generalized
bounds, though most of it will likely be optimized with function inlining. However, the savings
in avoiding copying and modifying keys to construct bounds might sufficiently offset these
overheads.

There is an existing related idea called Endpoint in transaction.h, but it is not nearly as general
or as well specified. (Lacking unit tests; not clear what reverse comparator semantics are; doesn't
add clarity to inclusive vs. exclusive bounds; unclear timestamp semantics)

See comparator.h changes for details.

Test Plan: TODO

The APIs mentioned in #11027 would likely be the first upgraded to support SliceBound.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants