feat(scheduler): range-repartition rule primitives (batteries-included) - #2196
Open
avantgardnerio wants to merge 2 commits into
Open
feat(scheduler): range-repartition rule primitives (batteries-included)#2196avantgardnerio wants to merge 2 commits into
avantgardnerio wants to merge 2 commits into
Conversation
Scheduler-side AQE machinery so any optimizer rule that inserts an `UnorderedRangeRepartitionExec` or `OrderedRangeRepartitionExec` gets end-to-end correct routing for free. A range-partitioning rule now needs to do only one thing: splice a `RuntimeStatsExec` + URRE/ORRE above whatever it wants range-repartitioned. Everything downstream is handled: - Executor already ships per-sub-part quantile sketches to the scheduler (apache#2094 / apache#2175). - Scheduler recognizes the range repartition at stage-completion time, merges sketches into `K − 1` global cuts, and rewrites the downstream location map so partition `k` pulls from every producer file whose sketched `[min, max]` overlaps `k`'s assigned cut range. - Cuts + routing expression are parked on the boundary `ExchangeExec`; at task-specialization time the adapter wraps the `ShuffleReader` in a `PerPartitionFilterExec` (apache#2195) that trims straddling sub-parts to their assigned slice. Primitives a rule writer now has: - `plan_contains_range_repartition` / `find_range_repartition_routing_expr` (runtime_stats.rs) — detect URRE or ORRE anywhere in a plan subtree. - `compute_overlapping_locations` / `overlap_remap_partitions` (runtime_stats.rs) — merged sketches + cuts → per-downstream-partition location map with correct producer-file assignment. - `ExchangeExec.range_repartition_routing` slot + `resolve_*` / `.range_repartition_routing()` accessors — scheduler → adapter handoff. Mirrors the existing `coalesce: Arc<Mutex<Option<Arc<CoalescePlan>>>>` slot pattern (same lifecycle, same `with_new_children` carry-through, same idempotent-overwrite semantics — see `b839f036` /`232d7611` for the coalesce refactors that established the pattern). - DER classifier arm (distributed_exchange.rs) — recognizes `RuntimeStatsExec → URRE/ORRE` as a stage boundary so the sketches ship to the scheduler at stage-N completion. All of this fires only when a rule has actually inserted an RRE. On plans without one, `plan_contains_range_repartition` returns `false` and the machinery no-ops — zero overhead for the 99%+ non-RRE case. Test coverage (15 new unit tests, all pure-function): - `plan_walker_tests` (6): URRE and ORRE at root and nested, bare source returns `None`/`false`. - `overlap_remap_tests` (5): disjoint, straddling, missing-file_id error, default/report mismatch error, empty-sketch passthrough. - `range_repartition_routing_tests` (4): unresolved returns `None`, resolve→get roundtrip, second resolve overwrites, `with_new_children` preserves the slot. Full end-to-end wiring is exercised in the follow-up consumer rule PRs (AdaptiveRangeShuffleRule for join-side rewrites, ParallelWindowDetectRule for parallel windows) where a live URRE-inserting fixture is naturally available; deferring the ~300 LOC of AQE-graph scaffolding here in favor of pure-function coverage. Existing tests: 222 in ballista-core, 281 in ballista-scheduler — all pass. Clippy clean, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@phillipleblanc this one is the base upon which anyone can build a range-repartitioned optimizer rule. I intend to use it for parallel windows in the next PR, but I was also playing with it to prevent spilling in SortShuffle on TPC-H q20. It's interesting because now we can play around and try things out. |
Rustdoc CI (`RUSTDOCFLAGS=-D warnings`) rejects public docs linking to private items. `range_repartition_common` is `mod` (not `pub mod`) and `split_batch_by_range` is `pub(super)`, so neither is reachable from the public rustdoc surface for `range_partition_predicates`. Convert the intra-doc link to plain text; the code cross-reference still reads for a curious reviewer grepping the tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
I won't have time to review this in depth for a couple of days, but I will look at it sometime this week. |
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.
Summary
I'm a little torn on this one.
pros:
Batteries-included AQE machinery so anyone can write a
UnorderedRangeRepartitionExec/OrderedRangeRepartitionExec-inserting optimizer rule and get correct end-to-end routing for free.A range-partitioning rule now needs to do only one thing: splice a
RuntimeStatsExec+URRE/ORREabove whatever it wants range-repartitioned. Everything downstream is handled:K − 1global cuts, and rewrites the downstream location map so partitionkpulls from every producer file whose sketched[min, max]overlapsk's assigned cut range.ExchangeExec; at task-specialization time the adapter wraps theShuffleReaderin aPerPartitionFilterExec(feat(core): add PerPartitionFilterExec — per-input-partition boolean predicate #2195) that trims straddling sub-parts to their assigned slice — correctness closes without the rule ever knowing about sketches, filters, or the AQE hook.cons:
This is lots of plumbing code to achieve that. I hope I've done it in the right places. Honestly piggy backing
range_repartition_routingonExchangeExecfeels the worst. However this is following the precedent set by thecoalescefield, so maybe it's okay?What a rule writer gets
plan_contains_range_repartitionfind_range_repartition_routing_exprorder_by[0].exprfrom the range repartitioncompute_overlapping_locations(reports, cuts)→ per-downstream-partition sub-part listoverlap_remap_partitionsVec<Vec<PartitionLocation>>remaprange_partition_predicates(routing_expr, cuts)→ K half-open range predicatesExchangeExec.range_repartition_routingslotRuntimeStatsExec → URRE/ORREas a stage boundaryAll of this fires only when a rule has actually inserted a range repartition. On any plan without one,
plan_contains_range_repartitionreturnsfalseand the machinery no-ops — zero overhead for the 99%+ non-range-repartition case.Design note: the
ExchangeExecrouting slotrange_repartition_routing: Arc<Mutex<Option<RangeRepartitionRouting>>>is a direct copy of the existingcoalesce: Arc<Mutex<Option<Arc<CoalescePlan>>>>slot pattern that landed withCoalescePartitionsRule— same lifecycle (AQE rule parks decision, adapter consumes at plan-transform time), samewith_new_childrencarry-through, same idempotent-overwrite setter. That slot went through review iteration on main (b839f036 refactor(scheduler): tighten the AQE coalesce rule after review,232d7611 let the AQE coalesce slot be cleared); this PR follows the pattern rather than inventing a new one.Example consumers
Two range-repartitioning rules are being prepared as follow-ups against this infrastructure:
AdaptiveRangeShuffleRule— rewrites Hash exchanges as URRE at join-side boundaries. Proof-of-concept for the machinery.ParallelWindowDetectRule(port frombrent/h2o-window-benchmarks) — wrapsBoundedWindowAggExecin URRE +RuntimeStatsExecfor parallelRANGE-frame windows. Motivating consumer for the h2o single-partition OOM case.Neither rule is in this PR — they'd conflate two decisions (the machinery vs. the specific consumer). Both are ready to send in short order once this lands.
Test plan
15 new unit tests covering the pure functions and the exchange slot:
plan_walker_tests(6): both URRE and ORRE recognized at root and nested, bare source returnsNone/false.overlap_remap_tests(5): disjoint routing, straddling replication, missing-file_id error, default/report mismatch error, empty-sketch passthrough.range_repartition_routing_tests(4): unresolved slot returnsNone, resolve→get roundtrip, second resolve overwrites,with_new_childrenpreserves the slot.Full end-to-end wiring (
maybe_range_repartition_overlap_remapfires at stage completion → cuts parked → adapter injects filter) is exercised in the consumer rule PRs where a live URRE-inserting fixture is available; deferring the ~300 LOC of AQE-graph scaffolding here in favor of pure-function coverage.Existing suites: ballista-core (222), ballista-scheduler (281) — all pass. Clippy clean, fmt clean.
Caveats
RangeRepartitionRoutingis scheduler-only state — no wire/codec changes.plan_contains_range_repartitionstops at exchange boundaries by design (a nested range repartition lives in a different stage anyway).🤖 Generated with Claude Code