peterxcli commented on PR #5493:
URL: 
https://github.com/apache/datafusion-comet/pull/5493#issuecomment-5452760069

   Addressed the [lazy-input cross-pool 
follow-up](https://github.com/apache/datafusion-comet/pull/5493#pullrequestreview-5051189659)
 in ab5c033ad with both halves of your suggestion, since neither alone closes 
it: deferral cannot help once rows are buffered (mid-write `hasNext()` on 
streaming input can block on Spark memory just as the first call can), and 
unwinding alone would leave the common window wider than necessary.
   
   1. **No retention across the lazy input boundary**: the sorter is now 
allocated only after `records.hasNext()` has produced the first record, so a 
task blocked in Spark execution memory while its input materializes holds zero 
Comet pool bytes.
   2. **The retry unwinds when the dependency cannot progress**: after at least 
one full wait interval, a Comet waiter throws the managed 
`SparkOutOfMemoryError` (as base did) when the memory it waits for is retained 
by threads parked in an *untimed* wait whose stack is inside 
`ExecutionMemoryPool`/`UnifiedMemoryManager` — i.e. blocked acquiring Spark 
execution memory, which may in turn only be freed by tasks blocked here. The 
detection is deliberately narrow so the previously-reviewed healthy-holder 
scenarios keep waiting: a sleeping holder is `TIMED_WAITING`, a latch-parked 
holder with an independent release has no Spark-memory frames, and a computing 
or I/O-bound holder is `RUNNABLE` — none match.
   
   Added a regression test that builds the real two-pool state: one task owns 
the whole Spark execution pool (a real `UnifiedMemoryManager`), a holder thread 
retains 500 KiB of the 1-MiB Comet pool and then genuinely parks inside 
`ExecutionMemoryPool` acquiring Spark memory (verified from its stack), and a 
Comet waiter requesting 800 KiB unwinds with `SparkOutOfMemoryError` instead of 
hanging; once Spark memory frees, the holder resumes and the pool drains fully. 
Also reworked the unsafe-writer test to assert nothing is allocated before the 
first record is consumed.


-- 
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]

Reply via email to