sunchao commented on code in PR #5493:
URL: https://github.com/apache/datafusion-comet/pull/5493#discussion_r3869474061
##########
spark/src/main/java/org/apache/spark/sql/comet/execution/shuffle/CometBypassMergeSortShuffleWriter.java:
##########
@@ -176,6 +177,10 @@ public void write(Iterator<Product2<K, V>> records) throws
IOException {
CometShuffleExternalSorter.MAXIMUM_PAGE_SIZE_BYTES,
memoryManager.pageSizeBytes()));
+ // This task's disk writers. Under memory pressure a writer spills its
sibling writers in
+ // this list, never those of other tasks.
+ final LinkedList<CometDiskBlockWriter> taskWriters = new LinkedList<>();
Review Comment:
[P2] Preserve progress for the shared on-heap shuffle pool
In the explicitly enabled on-heap compatibility mode,
`CometShuffleMemoryAllocator.getInstance()` still returns one
`CometBoundedShuffleMemoryAllocator` shared by the executor. If task B holds
that pool before A allocates its first page, A's new list contains nothing
spillable, so `initialCurrentPage()` now throws `SparkOutOfMemoryError`. I
reproduced this through a real Comet JVM shuffle on Spark 3.5.9 with 256-KiB
pages and a 1-MiB shuffle budget: B idles with four pages (751 binary rows),
then A fails on its first row. The same job with the two exact base Java
classes returns all 752 rows; this head also succeeds with a 2-MiB budget.
Please preserve safe allocation progress for this shared pool when isolating
the spill registries; the maintained on-heap Spark SQL compatibility path still
uses it.
--
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]