Victsm commented on a change in pull request #33613:
URL: https://github.com/apache/spark/pull/33613#discussion_r681848866
##########
File path:
common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ErrorHandlerSuite.java
##########
@@ -31,11 +31,11 @@
@Test
public void testErrorRetry() {
ErrorHandler.BlockPushErrorHandler pushHandler = new
ErrorHandler.BlockPushErrorHandler();
- assertFalse(pushHandler.shouldRetryError(new RuntimeException(new
IllegalArgumentException(
-
ErrorHandler.BlockPushErrorHandler.TOO_LATE_OR_STALE_BLOCK_PUSH_MESSAGE_SUFFIX))));
Review comment:
The reason being, in `TransportResponseHandler#handle` in case of a
`RPCFailure`, the server side exception stack trace is used to initialize the
RuntimeException which gets passed to all the client side RPCResponseCallback.
So the server side information we need to check is always in the first level
exception's message.
So, on the client side, we can switch from using
`Throwables.getStrackTraceAsString` to just check the exception's message.
On the server side, RemoteBlockPushResolver is also using ErrorHandler to
check if it needs to log certain exceptions.
We need to check both the exception and the cause of the exception there.
Of course, what you mentioned in the other comment makes sense, i.e.
assuming the exception structure is fragile.
--
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]
