dongjoon-hyun opened a new pull request, #59805: URL: https://github.com/apache/spark/pull/59805
### What changes were proposed in this pull request? This PR makes the two `logError` calls on the interrupt (kill) path of `TaskContextImpl` best effort: - In `invokeTaskInterruptListeners`, the per-listener `logError` in the loop is wrapped in `try`/`catch` (`Throwable`, like the catch it sits in and like the loops in #59794). A failure to log is added to the listener's exception as a suppressed exception, and the loop goes on with the remaining listeners. - In `markInterrupted`, the `logError` of the aggregate failure is wrapped in `try`/`catch` (`NonFatal`). A failure to log is added to the aggregate exception that `markTaskFailed(e)` records right after. `NonFatal` keeps the contract of SPARK-60554, where an error from `invokeListeners` itself (e.g. an `OutOfMemoryError`) still propagates. ### Why are the changes needed? With an appender that does not ignore exceptions (`ignoreExceptions=false`), log4j throws an `AppenderLoggingException` from the logging call. On the interrupt path this happens when an interrupt listener fails and logging its failure fails: - The loop ends, so the remaining interrupt listeners never run. - The exception escapes `markInterrupted` before `markTaskFailed(e)`, so the task is not marked failed (`isFailed` stays `false`) and its failure listeners do not run. - `Task.kill` calls `context.markInterrupted(reason)` before `taskThread.interrupt()`, so the task thread is never interrupted. SPARK-60554 (#59792) guarded only the `markTaskFailed(e)` call, and SPARK-60556 (#59794) guarded the pre-completion loop and `invokeListeners`. This PR covers the two remaining unguarded log calls on the interrupt path. This change overlaps textually with any rewrite of `invokeTaskInterruptListeners` that moves the recording of the aggregate failure to the end of the loop. Whichever lands second should carry the `try`/`catch` over to the moved `logError`. ### Does this PR introduce _any_ user-facing change? No API or configuration change. Only deployments that use a log appender that does not ignore exceptions are affected: a killed task whose interrupt listener fails now still runs the remaining interrupt listeners, is marked failed, and has its task thread interrupted. This behavior exists in released Spark [4.2.0](https://github.com/apache/spark/releases/tag/v4.2.0) (2026-07-11). ### How was this patch tested? Added three tests to `TaskContextSuite`. They reuse the `FailingAppender` added by #59794, an appender with `ignoreExceptions=false` that throws for log events whose message contains a trigger string: - `SPARK-60567: all TaskInterruptListeners should be called even if logging a failure fails` - `SPARK-60567: markInterrupted marks the task failed even if logging the listener failures fails` - `SPARK-60567: Task.kill interrupts the task thread even if logging the listener failures fails` Without the fix, all three fail: ``` [info] - ... *** FAILED *** [info] org.apache.logging.log4j.core.appender.AppenderLoggingException: An exception occurred processing Appender failingAppender ``` The exception is thrown from `invokeTaskInterruptListeners` (`TaskContextImpl.scala:323`), `markInterrupted` (`TaskContextImpl.scala:446`), and `Task.kill`, respectively. With the fix, they pass, including 10 consecutive runs. ``` build/sbt 'core/testOnly *TaskContextSuite *BarrierTaskContextSuite *ExecutorSuite *TaskSetManagerSuite' build/sbt core/scalastyle core/Test/scalastyle ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Sonnet 5.5 -- 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]
