Repository navigation
fix: keep what an in-memory output merge's precheck was granted, and overcommit a small shortfall - #25
Merged
Conversation
…overcommit a small shortfall The precheck added in #22 asked the pool for the memory the output merge of the buffered batches needs beyond the sorter's reservations, and gave it straight back. The workspace then kept only the merge headroom. In a Spark task that also runs a JVM sort and an UnsafeShuffleWriter (parse_product_default, stage 275 on joom-1.1-12), the JVM sort spilled for the precheck's request, the memory it freed went to the shuffle writer, which does not spill for other consumers, and the merge failed a few KB later with "Additional allocation failed for ExternalSorterMerge[0] ... can spill: false" (7 task failures; 0 on 1.1-9..1.1-11). - reserve_output_merge keeps the precheck's grant as a reservation, and the in-memory merge takes it into its workspace as a third parent. The workspace keeps max(headroom, needed - buffered) unused bytes, so the merge grows into memory it already holds. keep_at_most now runs after the runs are registered, so the sorter's reservation is no longer returned to the pool and grown back with the infallible grow. - A fallible growth the pool refuses is taken with the infallible grow while the total of such growth stays within max(16 MiB, 10% of the workspace), which the Comet pool carries as overcommit and repays from its next grant. Only the in-memory output merge allows it; spill merges keep their fan-in and read-ahead degradation, and the bounded view stays strict. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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?
Closes #. (Joom fork, OOM diagnosis of night 10-17 -> 10-18, fix C)
Rationale for this change
parse_product_defaulton joom-1.1-12, stage 275 (JVM Sort 856 GiB + CometSort 20.7 GiB -> JVM SortMergeJoin LeftAnti -> UnsafeShuffleWriter): 7 task failuresAdditional allocation failed for ExternalSorterMerge[0] ... ExternalSorterMerge consumed 80.6 MB (can spill: false), the failing task's ShuffleExternalSorter holding 1544 MiB. The same stage had 0 failures on 1.1-9..1.1-11.The output-merge precheck from #22 (
output_merge_fits) probed the pool withtry_growand released the grant immediately; the workspace then kept only the merge headroom. Memory released to Spark during the merge, and the probe's grant itself, end up with the shuffle writer, which never spills for another consumer, so the merge's next few-KB growth gets 0 bytes.What changes are included in this PR?
reserve_output_merge(wasoutput_merge_fits) returns the estimate and keeps the grant as a reservation; the in-memory merge takes it into itsSpillWorkspaceas a third parent and keepsmax(headroom, needed - buffered)unused bytes reserved, so it grows into memory it already holds.keep_at_mostnow runs after the runs are registered, so the sorter's reservation is no longer released and grown back.SpillWorkspace::allow_overcommit: for the in-memory output merge only, a fallible growth the pool refuses is taken with the infalliblegrowwhile the total stays withinmax(16 MiB, 10% of the workspace); the Comet pool carries it as overcommit and repays from the next grant. Spill merges keep their own degradation; the bounded view stays strict.How are these changes tested?
output_merge_keeps_what_its_precheck_was_granted(comet_memory_tests.rs): the precheck passes, then a second consumer takes the free pool and every released byte. On the base it fails withResourcesExhausted("Failed to allocate additional 566.4 KB for ExternalSorterMerge[0] ..."), the production signature; with the fix it passes, also with the overcommit allowance set to 0 (the retained grant alone is enough).refused_growth_overcommits_only_within_the_allowance(spill_workspace.rs).cargo fmt --check,cargo clippy --all-targets --workspace -D warningsclean.🤖 Generated with Claude Code