Skip to content

compute: thin peek results by partitioning, not sorting - #38041

Open
aljoscha wants to merge 1 commit into
aljoscha/peek-01-cooperativefrom
aljoscha/peek-02-thinning
Open

compute: thin peek results by partitioning, not sorting#38041
aljoscha wants to merge 1 commit into
aljoscha/peek-01-cooperativefrom
aljoscha/peek-02-thinning

Conversation

@aljoscha

@aljoscha aljoscha commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Motivation

Part 2 of 2 in a stack that makes index peeks stop monopolizing the compute
worker. Part 1 is #38040. A replica crash fix in the same code went first, in
#38039, and has landed.

When a peek's finishing bounds how many rows it can need, the scan keeps twice
that many and periodically drops the excess. It did so by sorting the whole
buffer and truncating. That costs a log factor per row for an order that is
thrown away: the result is ordered once at the end, when the RowCollection is
built. Each comparison decodes both rows, so for a peek with an ORDER BY and
a small LIMIT over a large arrangement this is the dominant cost.

Part of CPU-195.

Description

select_nth_unstable_by gives the same retained rows in O(n) comparisons
instead of O(n log k).

The partition is not stable, so when rows tie across the cut it is unspecified
which of them survives. That is unobservable, and the argument is worth stating
because it is the only thing making this a safe swap:

  • A tie under RowComparator::compare_rows(l, r, || l.cmp(r)) means the
    order_by columns compare equal and the tiebreaker compares equal, and the
    tiebreaker is a full comparison of the encoded rows. So tied rows are
    byte-identical.
  • The retained run therefore agrees with any other choice on its first
    limit + offset rows, whichever way the cut lands.
  • RowSetFinishing::finish reads exactly offset..offset + limit of the merged
    result, and merge_sorted's output prefix depends only on the runs' prefixes.
    So the client sees the same rows either way.

mz_index_peek_result_sort_seconds is renamed to
mz_index_peek_result_thinning_seconds, since it no longer times a sort. The
one remaining sort, when the RowCollection is built, is timed by
mz_index_peek_row_collection_seconds.

Not changed on purpose. The unconditional sort in RowCollection::new,
which fires even when order_by is empty, stays. It is load-bearing for
determinism, not for correctness: per-worker peek responses are absorbed in
arrival order by a randomized StreamMap, so sorting each run by encoded bytes
plus a byte-order k-way merge in envd is what makes client-visible row order
reproducible independent of worker count and arrival order. Roughly 1100
multi-row, no-ORDER BY, nosort sqllogictest goldens ride on it, and
test/sqllogictest/range.slt:359 pins the length-first-then-bytes encoding
order exactly.

Verification

Covered by the existing suite: any change to which rows survive thinning, or to
their order, shows up in the sqllogictest goldens described above, since they
compare output positionally.

@aljoscha
aljoscha requested review from a team as code owners August 4, 2026 11:54
@linear-code

linear-code Bot commented Aug 4, 2026

Copy link
Copy Markdown

CPU-195

@aljoscha
aljoscha requested a review from a team as a code owner August 4, 2026 12:14
@aljoscha
aljoscha force-pushed the aljoscha/peek-02-thinning branch from 6fc9408 to 659e036 Compare August 4, 2026 12:14
@aljoscha
aljoscha requested a review from DAlperin August 4, 2026 13:52
@aljoscha
aljoscha force-pushed the aljoscha/peek-02-thinning branch from 659e036 to b6fa502 Compare August 4, 2026 18:04
When a peek's finishing bounds how many rows it can need, the scan keeps
twice that many and periodically drops the excess. It did so by sorting
the whole buffer and truncating, which costs a log factor per row for an
order that is thrown away: the result is ordered once at the end, when
the `RowCollection` is built.

Partitioning gives the same retained rows in O(n) comparisons. Each
comparison decodes both rows, so this is the dominant cost of a peek with
an `ORDER BY` and a small `LIMIT` over a large arrangement.

The partition is not stable, so when rows tie across the cut it is
unspecified which survives. That is unobservable: a tie under this
comparator means the rows are byte-identical, so whichever way the cut
lands the retained run agrees with any other choice on its first
`limit + offset` rows, and that prefix is all the finishing reads.

`mz_index_peek_result_sort_seconds` becomes
`mz_index_peek_result_thinning_seconds`, since it no longer times a sort.
The one remaining sort, when the `RowCollection` is built, is timed by
`mz_index_peek_row_collection_seconds`.
@aljoscha
aljoscha force-pushed the aljoscha/peek-02-thinning branch from b6fa502 to d450e69 Compare August 4, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant