fix: [branch-1.1] log partial memory grants at DEBUG and drop the memory usage dump (#6269) - #6346
Merged
Conversation
apache#6269) CometTaskMemoryManager.acquireMemory logged a warning, then called TaskMemoryManager.showMemoryUsage, every time Spark granted less memory than a native pool asked for. A partial grant is how a native operator learns to spill, so a query that spills logged hundreds of them. Log the partial grant at DEBUG instead. A refusal that fails the task already says in its error what Spark granted and lists the pool's top consumers. Drop the dump rather than moving it to DEBUG. showMemoryUsage takes the TaskMemoryManager monitor. Another acquire of the same task can hold that monitor while it waits inside Spark for memory, and this thread still holds its partial grant. The task then hangs until some other task frees memory. greedy_unified can reach this on main, because it calls Spark without a lock. sunchao found the cycle while reviewing apache#5613. Closes apache#6257. (cherry picked from commit e1d2c11)
github-actions
Bot
added
bug
comphead
reviewed
Sep 28, 2026
| long newUsed = used.addAndGet(acquired); | ||
| if (acquired < size) { | ||
| logger.warn( | ||
| // A partial grant is routine, not an error: the native pool either refuses the reservation, |
Contributor
There was a problem hiding this comment.
I feel this comment can be simplified
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
2 participants
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.
Backport of #6269 to
branch-1.1.Cherry-picked from
e1d2c11729c2fc60a5def4e87bb17e5b28df2a29without conflicts. The three files it changes are identical onbranch-1.1and onmainjust before #6269, so the diff is byte-identical to upstream.Which issue does this PR close?
Closes #6257 on
branch-1.1. #6269 already closed it onmain.Rationale for this change
#6269 merged after
branch-1.1was cut at 36ab57c, so without this 1.1.0 ships both of the problems it fixes:CometTaskMemoryManager.acquireMemorylogs a warning and then callsTaskMemoryManager.showMemoryUsage(), which logs at least three more lines at INFO. A partial grant is routine under memory pressure, since it is how a native operator learns to spill, so a query that spills floods the executor log.showMemoryUsage()takes theTaskMemoryManagermonitor. Another acquire of the same task can hold that monitor while it waits inside Spark for memory, and the thread that got the short grant keeps those bytes until it returns to native code. The task then hangs until some other task frees memory. The memory pools onbranch-1.1are the same as onmain, sogreedy_unified, which calls Spark without a lock, can reach this. The defaultfair_unifiedholds its lock across the call, which rules the cycle out between two native threads of a task; fix: log partial memory grants at DEBUG and drop the memory usage dump #6269 has the details. sunchao found the cycle while reviewing perf: stop holding the fair pool lock across blocking memory calls #5613.Only logging changes. A partial grant is now logged at DEBUG, and the memory dump is gone. A reservation that really fails still says in its error how much Spark granted and which consumers hold the most memory.
What changes are included in this PR?
The original change, so see #6269 for the details. No adaptations were needed. In short:
acquireMemorylogs a partial grant at DEBUG, behindisDebugEnabled(), and no longer callsshowMemoryUsage(). A comment says why the method must not take theTaskMemoryManagermonitor.CometTaskMemoryManagerSuite, which now extendsSparkFunSuiteso that it can usewithLogAppender.How are these changes tested?
The original PR's tests, run locally on
branch-1.1with JDK 17:CometTaskMemoryManagerSuitepasses, 5 tests, on the default profile (Spark 4.1, Scala 2.13) and on Spark 3.4 with Scala 2.12. The build's spotless and scalastyle checks ran and passed in both.CometTaskMemoryManager.javareverted tobranch-1.1's copy, both new tests fail. The INFO-level test captures two warnings and eight dump lines, the same as fix: log partial memory grants at DEBUG and drop the memory usage dump #6269 reported onmain.Against
branch-1.1, the changed paths route this pull request to every suite except Spark 3.4's SQL job, the PyArrow UDF job and the benchmark check.