Copilot commented on code in PR #12496:
URL: https://github.com/apache/gluten/pull/12496#discussion_r3557947358
##########
gluten-core/src/main/scala/org/apache/spark/task/TaskResources.scala:
##########
@@ -104,8 +104,8 @@ object TaskResources extends TaskListener with Logging {
try {
context.markTaskFailed(t)
} catch {
- case t: Throwable =>
- t.addSuppressed(t)
+ case markFailure: Throwable =>
+ t.addSuppressed(markFailure)
Review Comment:
This change fixes the suppressed-exception chain on a rare double-failure
path (body throws and `context.markTaskFailed(t)` also throws), but there’s
currently no regression test exercising that behavior. There are already unit
tests covering `TaskResources.runUnsafe` (e.g.,
`gluten-core/src/test/scala/org/apache/gluten/task/TaskResourceSuite.scala`),
so it should be feasible to add one that forces `markTaskFailed` to throw (for
example, by registering a `TaskFailureListener` that throws) and then asserts
the original exception contains the follow-up failure in `getSuppressed`.
--
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]
