Conversation
The tuning guide opened the memory section with "specify how much memory Comet can use in addition to memory already allocated to Spark", which reads as though Comet's budget sits outside what the container already accounts for. That is the misconception apache#6014 and apache#6029 established the contributor-guide diagram to prevent, and users are the audience that needs it. Adds the container sizing sum, states the two facts against it (offHeap.size is inside the limit; memoryOverhead is the only slack and sizes no budget), and compares how the three settings that respond to an OOM kill move the container and Comet's budget in different directions. Adds a table of what each of the three budgets looks like when it is exceeded, notes that fair_unified is currently stricter than documented (apache#5961), and links the contributor guide's Memory Management page.
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 #6032.
Rationale for this change
#6014 and #6029 established, in the contributor guide, where Comet's memory actually lives and which configuration value sizes each part of the executor container. Two of those facts are the ones users get wrong, and neither appeared anywhere in the tuning guide:
spark.memory.offHeap.sizeis part of the container budget rather than headroom on top of it, andspark.executor.memoryOverheadis the container's only slack while sizing no budget any consumer can draw on.The tuning guide opened the memory section with "specify how much memory Comet can use in addition to memory already allocated to Spark", which reads as though Comet's budget sits outside what the container already accounts for. That is exactly the misconception the contributor-guide diagram was changed to prevent, and the audience that needs it most does not read the contributor guide.
The practical consequence was that the guide gave no way to choose between the three settings that respond to an executor OOM kill, and they do not behave alike. Raising
spark.executor.memoryOverheadwidens the container and leaves Comet's budget alone. Loweringspark.comet.exec.memoryPool.fractionleaves the container alone and shrinks what Comet may reserve. Raisingspark.memory.offHeap.sizedoes both, which makes it the intuitive response and the least reliable one, since it also raises the ceiling on the unaccounted overshoot that caused the kill. The guide also never said what an OOM looks like from each of the three budgets, so there was nothing to tell a user whether the setting they were about to change was the one that overflowed.What changes are included in this PR?
All in
docs/source/user-guide/latest/tuning.md:OOMKilled), JVM heap exhaustion (exit 52), and a task-levelSparkOutOfMemoryError, with what to change in each case and which settings show what the pool was doing.fair_unifiedis currently stricter than the paragraph describing it, pointing at fair_unified memory pool caps the pool total at pool_size / num_consumers instead of each consumer's share #5961. The existing description is left in place rather than rewritten, because fair_unified memory pool caps the pool total at pool_size / num_consumers instead of each consumer's share #5961 is a regression: when it is fixed the note is simply deleted.The container sizing sum is Spark's own, from
ResourceProfile.getResourcesForClusterManager, and matches the one already stated in the contributor guide.One thing this PR deliberately does not touch: the TPC-H figures in Determining How Much Memory to Allocate ("Comet requires at least 5 GB of RAM") do not say which setting the number refers to, and I did not want to guess at a reading of numbers I cannot re-derive. Worth a separate look.
How are these changes tested?
Docs only, no code paths touched. No mermaid fences are added, so the #6021 pre-render step is unaffected.
npx prettier "docs/source/user-guide/latest/tuning.md" --checkpasses, which is what the Preflight job runs.The two new in-page anchors follow the slug form MyST generates with
myst_heading_anchors = 4, matching the existing#where-comets-budget-comes-fromlink inmemory_management.md.