sunchao commented on code in PR #5494:
URL: https://github.com/apache/datafusion-comet/pull/5494#discussion_r3869989678
##########
spark/src/main/scala/org/apache/comet/CometExecIterator.scala:
##########
@@ -227,13 +227,33 @@ class CometExecIterator(
def close(): Unit = synchronized {
if (!closed) {
- if (currentBatch != null) {
- currentBatch.close()
- currentBatch = null
+ closed = true
+
+ var failure: Throwable = null
+ try {
+ if (currentBatch != null) {
+ currentBatch.close()
+ currentBatch = null
+ }
+ nativeUtil.close()
+ shuffleBlockIterators.values.foreach(_.close())
Review Comment:
[P2] Attempt cleanup of every shuffle input after a close failure
If an earlier shuffle input's `close()` throws, this `foreach` skips all
later inputs, and the new `closed = true` makes the task-completion retry a
no-op. This matters when an iterator closes before its raw shuffle inputs are
drained: `SequenceInputStream.close()` can fail while fetching/closing
remaining blocks, while another input already owns an open file stream. Spark's
completion cleanup only calls `FileSegmentManagedBuffer.release()` (a no-op for
its `FileInputStream`), and dropping the native plan does not invoke Java
stream close, so the descriptor stays open until GC.
I reproduced this on Spark 4.0.4/JDK 17 with two real
`CometShuffleBlockIterator`s, `SequenceInputStream`s and a file-backed managed
buffer: after the first close throws and task completion runs, the later
descriptor remains open at this head. With only the iterator class replaced by
the exact base version, task completion closes it; normal cleanup passes both
controls. Please attempt each resource's cleanup independently, accumulating
exceptions while still releasing the native plan exactly once.
--
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]
