Repository navigation
refactor: one admission rule for every spill merge - #26984
Draft
jayzhan211 wants to merge 4 commits into
Draft
jayzhan211 wants to merge 4 commits into
jayzhan211 wants to merge 4 commits into
Conversation
jayzhan211
force-pushed
the
poc/spill-batch-bound
branch
from
November 1, 2026 06:12
97b4c1f to
ba3c3da
Compare
This was referenced Nov 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Stacked on #26987. Only the last commit, "refactor: one admission rule for every spill merge", is new here; I'll rebase once #26987 merges.
Rationale for this change
After #26987, the spill merge still has two options that only the aggregate sets:
with_replay_headroom(#25383) andwith_intermediate_merge_sizing(#25428). #26711 asks to remove options like these, and #26264 proposes a third,with_fan_in_headroom, so that a spilled sort also leaves memory for other operators.This PR replaces all three with one rule that every caller gets: each run a merge reads must leave as much again as the merge takes beyond the workspace it already holds. This is the formula from #26264.
sort_spill_reservation_bytes, so only what it takes beyond that is doubled.What changes are included in this PR?
max_spill_merge_fan_in, replacing the full-batch and uniform-run checks.with_replay_headroomandwith_intermediate_merge_sizingon both merge builders,AggregateSpill's minimum batch-row tracking, and the row count returned by the intermediate spill.What is the testing strategy for this PR?
memory_limittests, extended fuzz tests, and the full sqllogictest suite.sort_tpchunder memory limits, which is needed to judge the cost below.sort_spill_reservation_bytes= 1 MB, and from 7/40 to 35/40 at 3 MB.Are there any user-facing changes?
Query results don't change. A spilled sort's merge now leaves headroom beyond its workspace, as #26264 proposes. This applies even when nothing else is using memory, so it can add merge passes. Sorting 3M rows by an integer and a short string on one partition, with
sort_spill_reservation_bytes= 4 MB:Intermediate merge sizing now applies to sorts as well as aggregates.