zuston commented on code in PR #1411:
URL:
https://github.com/apache/incubator-uniffle/pull/1411#discussion_r1451386753
##########
internal-client/src/main/java/org/apache/uniffle/client/impl/grpc/ShuffleServerGrpcClient.java:
##########
@@ -461,7 +462,7 @@ public RssSendShuffleDataResponse
sendShuffleData(RssSendShuffleDataRequest requ
maxRetryAttempts,
t -> !(t instanceof OutOfMemoryError));
} catch (Throwable throwable) {
- LOG.warn(throwable.getMessage());
+ LOG.warn(ExceptionUtils.getStackTrace(throwable));
Review Comment:
`LOG.warn("Failed to require pre-allocated memory or send shuffle data due
to ", throwable);` +1
##########
client-spark/spark3/src/main/java/org/apache/spark/shuffle/DelegationRssShuffleManager.java:
##########
@@ -167,7 +168,7 @@ private boolean tryAccessCluster() {
"Fail to access cluster {} using {} for {}",
coordinatorClient.getDesc(),
accessId,
- e.getMessage());
+ ExceptionUtils.getStackTrace(e));
Review Comment:
Got it. But I prefer using `e` directly
##########
common/src/main/java/org/apache/uniffle/common/util/RetryUtils.java:
##########
@@ -76,10 +76,15 @@ public static <T> T retryWithCondition(
Function<Throwable, Boolean> isRetryFunc)
throws Throwable {
int retry = 0;
+ Throwable previousThrowable = null;
while (true) {
try {
return cmd.execute();
} catch (Throwable t) {
+ if (previousThrowable != null &&
!previousThrowable.getClass().equals(t.getClass())) {
Review Comment:
I got your point. But it looks strange, simetimes the class maybe same, but
the underlying exception messages are different.
How about the way of using debug log level to show the complete stacktrace?
##########
internal-client/src/main/java/org/apache/uniffle/client/impl/grpc/ShuffleServerGrpcNettyClient.java:
##########
@@ -154,7 +155,7 @@ public RssSendShuffleDataResponse
sendShuffleData(RssSendShuffleDataRequest requ
maxRetryAttempts,
t -> !(t instanceof OutOfMemoryError));
} catch (Throwable throwable) {
- LOG.warn(throwable.getMessage());
+ LOG.warn(ExceptionUtils.getStackTrace(throwable));
Review Comment:
`LOG.warn("Failed to send shuffle data due to ", throwable);` +1
--
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]
