viirya commented on PR #7042:
URL:
https://github.com/apache/datafusion-comet/pull/7042#issuecomment-6220324915
Thanks @andygrove and @rich7420, you're right that the same-thread
assumption doesn't hold. `RecordBatchReceiverStream` and
`SpawnedTask::join_unwind` resume a spawned task's panic on the consuming
thread with `resume_unwind`, which skips the hook. That covers
`CoalescePartitionsExec`, and `SortPreservingMergeExec` and the spill reader
through `spawn_buffered`. `RepartitionExec` isn't one of them: its
`wait_for_task` joins the input task with `join()` and sends the panic on as a
`DataFusionError` ("Join Error"), so it never becomes `CometError::Panic`,
before or after this change.
I went with no backtrace rather than a process-wide fallback, since the
fallback would bring back #7041. The latest commit:
- clears the captured panic of the current thread at the start of
`try_unwrap_or_throw` and `catch_native`, so a backtrace left by a panic
handled before the call, on a pooled thread or otherwise, is never used;
- has the hook record the panic message with the backtrace, and only
attaches the backtrace when the message matches the `CometError::Panic` being
reported. That also covers panics caught during the call by `catch_unwind`
outside Comet, where clearing at each catch site isn't possible.
A resumed panic now reaches the JVM with its message and no stack trace. Its
location is still in the executor's stderr, where the hook logs every panic on
the thread that raised it.
New tests resume a panic from another thread inside `catch_native` with an
empty slot, after a panic handled during the call, and after a handled panic
with the same message before the call, and check that none of them gets a
backtrace. Removing the message check fails the second, and removing the clear
fails the third. I also updated the description.
--
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]
