refactor: track NestedLoopJoin fallback matches in one left bitmap and drop the cancel protocol - #25542
Open
jayzhan211 wants to merge 3 commits into
Open
refactor: track NestedLoopJoin fallback matches in one left bitmap and drop the cancel protocol#25542jayzhan211 wants to merge 3 commits into
jayzhan211 wants to merge 3 commits into
Conversation
…d, bitmap copy, stale doc)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25542 +/- ##
========================================
Coverage 82.39% 82.39%
========================================
Files 1138 1138
Lines 434592 434390 -202
Branches 434592 434390 -202
========================================
- Hits 358073 357910 -163
+ Misses 54853 54839 -14
+ Partials 21666 21641 -25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…d drop the cancel protocol
jayzhan211
force-pushed
the
nlj-shared-left-bitmap
branch
from
September 20, 2026 14:28
1f13f0d to
c367337
Compare
jayzhan211
marked this pull request as ready for review
September 20, 2026 14:35
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?
Rationale for this change
Thanks @viirya for #22038 and #25004, which made
LEFT/FULL/LEFT SEMI/LEFT ANTI/LEFT MARKnested loop joins spill with a multi-partition right side instead of failing. This PR keeps that capability, keeps the way left chunks are scheduled, and changes how matches are tracked.Two user-visible problems today:
HashJoinExec, the others simply finish. fix: cancel the NestedLoopJoin coordinated fallback when a partition is dropped unfinished #25004 asked reviewers to weigh in on "letting survivors finish"; nobody did, so I am picking that question up here.nested_loop_join.rs(roughly 300 of production code, the rest tests and test seams) exist only to make that early drop safe:cancel_notify, registered watchers polled on everypoll_next,pending_drop/coordination_started, aselect!racing each chunk load against cancellation, a publish guard, andcancelled_terminally.Both come from one coupling. On the in-memory path the shared probe counter only decides who emits the unmatched left rows. In the fallback the same per-chunk counter also decides when the next chunk may load, and the partition it elects holds the only
release_chunkcall while it emits. A partition that goes away can therefore strand the chunk, and the loader going away takes the shared left stream with it.What changes are included in this PR?
Three commits, meant to be reviewed in order. It will be squashed on merge; the split is only there to make the last one easier to read.
1.
test:coverage that does not depend on this changeThese pass on
mainas well, so they show the same behaviour before and after.test_nested_loop_join_spill_fuzz: all 10 join types, right side spread over 4 partitions, comparing a run under a 4 KB limit with the same join without a limit, and asserting the limited run spilled. Until now no fuzz test reached the NLJ fallback.nested_loop_join_spill.slt: multi-partitionLEFT MARKandLIMITcases.memory_limit/nlj_spill_unmatched.rs: assertspill_count > 0exactly when a memory limit is set. These three regressions would otherwise keep passing if the join stopped spilling.2.
chore:small cleanups, no behaviour changeexecute()computed the "can this join spill" condition twice, with the same long comment; compute it once.left_buffered_in_one_pass, which is written in three places and never read.process_left_unmatched_rangecopied the visited bitmap bit by bit under anassert!whose message is"DBG: .."; useBooleanBuffer::collect_bool.3.
refactor:separate match tracking from chunk schedulingThe schedule does not change. All partitions still share one left chunk at a time, as large as memory allows, and the next chunk loads once every partition has finished the current one. That is what keeps right-side replays at a minimum, and it is the part of the current design worth keeping (see the measurements below).
Match tracking moves to one bitmap over the whole spilled left side, with one final emission pass. This is the mirror image of what #24746 did for the right side (
global_right_bitmaps+EmitGlobalRightUnmatched), and it is what other engines do: DuckDB'sPhysicalNestedLoopJoinkeeps oneOuterJoinMarkerover the whole build side and emits unmatched build rows in a separate final scan; Spark'sBroadcastNestedLoopJoinExecORs per-task bitsets into one and emits as a final step.LeftSpillData, already shared by all partitions throughOnceAsync<LeftLoad>, gets the bitmap (one bit per spilled left row, addressed by the row's position in the spill file) and the probe-threads counter — the same two thingsJoinLeftDatacarries on the in-memory path.ProbeEndis entered once per stream, after its last chunk. The last stream to report streams the left spill file once more and emits the final left rows batch by batch.With no per-chunk emitter, what is left of the coordinator is a count,
LeftChunkBarrier:finished == livemeans move on to the next chunk.OnceFutfor the whole left side. Whichever waiting partition is polled drives it and an error reaches all of them, so it no longer matters which partition started the load or whether it is still around.Decision,loader_in_flight,abandon_loadand theselect!go away.Donedeparts:live -= 1(andfinished -= 1if it had already finished the current chunk). The others carry on. Because the departed stream never reports probe completion, the final left rows are not emitted — exactly what happens on the in-memory path.nested_loop_join.rsgoes from 7,547 to 6,226 lines: production code −262, tests −1,059 (24 cancellation tests and their helpers are replaced by three drop scenarios).What I tried first, and why this PR looks different from its first version
The first version of this PR also gave every partition its own left chunk, so that partitions never wait for each other. It was simpler still, but it costs what the shared chunk saves: memory split P ways means chunks P times smaller and P times more right-side replays. On a wide-row workload (below)
mainmakes 7 passes over the right side and that version made 98, for about 1.6× the time and 1.5× the RSS. A variant sharing chunks through futures without a barrier reached time parity only at one hand-tuned chunk size, and nevermain's memory. A partition that runs ahead has to either wait or hold a second chunk; I kept the wait.What is the testing strategy for this PR?
mainand after this change.LEFTandFULLhad multi-chunk coverage)...._dropped_partition_does_not_stall_peers), dropped after finishing the current chunk while waiting for the others (..._partition_dropped_while_waiting_for_peers, which also checks the barrier's counts), and dropped right after starting a chunk load (..._partition_dropped_while_loading).test_nlj_memory_limited_releases_memory_{left,full}_join: nothing stays reserved after completion while the plan is still alive.--features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption): 11,989 passed, 0 failed.datafusion-clirelease builds of this branch and of themaincommit it is based on, 16 partitions, 24 MB limit,LEFT JOINwith a 50 MB build side (50K rows with a 1 KB string), interleaved runs on a machine that was not idle:maininput_rows/ 60K)I read that as no measurable difference in time and RSS within about 5%.
Are there any user-facing changes?
datafusion.execution.enable_nlj_coordinated_fallbackand its documentation are untouched.