peterxcli commented on PR #5493: URL: https://github.com/apache/datafusion-comet/pull/5493#issuecomment-5449245774
Addressed the [pre-write orphan finding](https://github.com/apache/datafusion-comet/pull/5493#pullrequestreview-5047933186) in 844b30867, taking the "give allocations task-completion cleanup" option: `CometUnsafeShuffleWriter`'s constructor now registers a `TaskCompletionListener` that runs `sorter.cleanupResources()` if the sorter is still alive. Completion listeners run in `Task.run`'s `finally` regardless of whether the failure was an Exception or an Error, so this covers the window you found — a fatal error thrown while Spark evaluates the input iterator, after the sorter allocated its pointer array but before `write()`'s own cleanup scope begins — as well as any other path that skips `stop(false)`. `cleanupResources()` is idempotent (the pointer array free is guarded, page lists are cleared, spill-file deletion checks existence), so it is a no-op on every normal path where `write()` or `stop()` already cleaned up. I did not defer the allocation instead, since the sorter's insert paths assume the array exists and lazy allocation would move the failure into a hotter path. Added a regression test that constructs the writer exactly as Spark does pre-iterator, asserts the pointer array actually occupies the 1-MiB pool (a full-pool allocation fails), then calls `markTaskCompleted` without `write()` or `stop()` ever running and asserts the full-pool allocation now succeeds — which fails without the listener. -- 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]
