peter-toth opened a new pull request, #58448: URL: https://github.com/apache/spark/pull/58448
### What changes were proposed in this pull request? Backport of #56293 to `branch-4.2`. Clean cherry-pick of `80554625ed6`, the `branch-4.3` commit, with no tailoring: `branch-4.2` already carries SPARK-56511 and its follow-up, which the change depends on. For `TaskMemoryManager`: - Replace recursive page allocation with bounded iterative recovery. - After allocator OOM, spill registered task-memory consumers directly without acquiring another fair-share execution-memory grant. - Measure progress using the consumer's tracked memory before and after spilling. - Retry the original grant while spilling makes progress, then attempt a smaller page based on the memory released. - Preserve one bounded additional execution-memory acquisition so callers can still receive a usable partial page from a free-tail grant. - Prevent page allocations made from inside either recovery spilling path from recursively entering allocator recovery. - Track acquired-but-unused portions of successful partial pages and make cleanup idempotent. - Mark pages returned by the final minimum-size retry after allocator failure so callers can distinguish them from ordinary exact-fit grants. The direct spill path can reset `ShuffleExternalSorter` while record insertion is in progress, so the change also makes its pointer-array lifecycle safe for that recovery path, and `BytesToBytesMap` and the Python `HybridQueue` reject exact-fit minimum-retry pages instead of consuming one page-table slot per record. See #56293 for the full description. ### Why are the changes needed? An execution-memory grant does not guarantee that the Tungsten allocator can create the page. When the allocator throws `OutOfMemoryError`, `TaskMemoryManager.allocatePage()` on this branch retains the grant as acquired-but-unused and calls itself recursively, with no depth bound and no progress check. Each retry asks for another grant while still holding the previous one. Two ways that ends badly: - Under sustained memory pressure the recursion runs deep enough to exhaust the thread's stack, and the executor dies with `StackOverflowError` instead of an `OutOfMemoryError` that task-level handling could report and retry. We have seen this in production with 240+ repeated `allocatePage` frames. The overflow also buries the allocation failure that caused it, which makes these failures hard to diagnose. - The task pins an increasing amount of execution memory and can end up blocking for more of it, far away from the original allocator OOM. SPARK-39897 has tracked the `StackOverflowError` since 2022 and SPARK-28314 reported it before that, so this is a long-standing crash rather than a new one. SPARK-57242's affects field says 4.3.0, but that understates it: the recursive retry has been in `TaskMemoryManager` for years and `branch-4.2` has it unchanged. Nothing on #56293 or on the JIRA argued against a maintenance backport, it just never came up - the PR was merged while 4.2.0 was in RC. That is the case for taking it here: 4.2.x is the newest released line, the fix is already on `branch-4.3` and `master`, and the pick is clean. ### Does this PR introduce _any_ user-facing change? Yes, on the allocator-OOM path only. Normal page allocation never enters the recovery loop. - A task that used to die with `StackOverflowError`, or block waiting for execution memory it had already pinned, now fails cleanly with `SparkOutOfMemoryError` once the bounded attempts are exhausted, so the scheduler can retry it. - The allocator-OOM WARN messages change: `Failed to allocate a page (N bytes), try again.` becomes `Failed to allocate a page (N bytes), try spilling task memory.` on the first failure and `Failed to allocate a page (N bytes) after K spill retries.` afterwards. No new configuration and no public API change. ### How was this patch tested? The tests that ship with #56293, run on this branch, all green: - `TaskMemoryManagerSuite`, 28 tests - `ShuffleExternalSorterSuite`, 9 tests - `ShuffleInMemorySorterSuite` and `ShuffleInMemoryRadixSorterSuite`, 6 each - `UnsafeShuffleWriterSuite`, 30 tests - `BytesToBytesMapOnHeapSuite` and `BytesToBytesMapOffHeapSuite`, 16 each - `UnsafeExternalSorterSuite`, 26 tests - `RowQueueSuite`, 8 tests The diff is byte-identical to `80554625ed6`, so no new test was added for this branch. ### Was this patch authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
