[#1267][followup] improvement(client): The previous exception may be …

…discarded when OOM occurs
apache · Jan 13, 2024 · 3b9d9fe · 3b9d9fe
1 parent 3194717
commit 3b9d9fe
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 13 deletions.
diff --git a/client-spark/spark3/src/main/java/org/apache/spark/shuffle/DelegationRssShuffleManager.java b/client-spark/spark3/src/main/java/org/apache/spark/shuffle/DelegationRssShuffleManager.java
@@ -164,10 +164,7 @@ private boolean tryAccessCluster() {
         return canAccess;
       } catch (Throwable e) {
         LOG.warn(
-            "Fail to access cluster {} using {} for {}",
-            coordinatorClient.getDesc(),
-            accessId,
-            e.getMessage());
+            "Fail to access cluster {} using {} for ", coordinatorClient.getDesc(), accessId, e);
       }
     }
 

diff --git a/common/src/main/java/org/apache/uniffle/common/util/RetryUtils.java b/common/src/main/java/org/apache/uniffle/common/util/RetryUtils.java
@@ -76,24 +76,26 @@ 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) {
         retry++;
         if (isRetryFunc.apply(t) && retry < retryTimes) {
-          previousThrowable = t;
-          LOG.info("Retry due to Throwable, " + t.getClass().getName() + " " + t.getMessage());
-          LOG.info("Waiting " + intervalMs + " milliseconds before next connection attempt.");
+          if (LOG.isDebugEnabled()) {
+            LOG.error("Retry due to Throwable ", t);
+          } else {
+            LOG.error(
+                "Retry due to Throwable {}. Use DEBUG level to see the full stack: {}",
+                t.getClass().getName(),
+                t.getMessage());
+          }
+          LOG.error("Will retry {} more time(s) after waiting {} milliseconds.", retryTimes - retry, intervalMs);
           Thread.sleep(intervalMs);
           if (callBack != null) {
             callBack.execute();
           }
         } else {
-          if (previousThrowable != null) {
-            LOG.error("The previous retry failed due to ", previousThrowable);
-          }
           throw t;
         }
       }

diff --git a/...nal-client/src/main/java/org/apache/uniffle/client/impl/grpc/ShuffleServerGrpcClient.java b/...nal-client/src/main/java/org/apache/uniffle/client/impl/grpc/ShuffleServerGrpcClient.java
@@ -461,7 +461,7 @@ public RssSendShuffleDataResponse sendShuffleData(RssSendShuffleDataRequest requ
             maxRetryAttempts,
             t -> !(t instanceof OutOfMemoryError));
       } catch (Throwable throwable) {
-        LOG.warn(throwable.getMessage());
+        LOG.warn("Failed to send shuffle data due to ", throwable);
         isSuccessful = false;
         break;
       }

diff --git a/...lient/src/main/java/org/apache/uniffle/client/impl/grpc/ShuffleServerGrpcNettyClient.java b/...lient/src/main/java/org/apache/uniffle/client/impl/grpc/ShuffleServerGrpcNettyClient.java
@@ -154,7 +154,7 @@ public RssSendShuffleDataResponse sendShuffleData(RssSendShuffleDataRequest requ
             maxRetryAttempts,
             t -> !(t instanceof OutOfMemoryError));
       } catch (Throwable throwable) {
-        LOG.warn(throwable.getMessage());
+        LOG.warn("Failed to send shuffle data due to ", throwable);
         isSuccessful = false;
         break;
       }