You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "tedyu (via GitHub)" <gi...@apache.org> on 2023/01/21 03:56:42 UTC

[GitHub] [spark] tedyu opened a new pull request, #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

tedyu opened a new pull request, #39654:
URL: https://github.com/apache/spark/pull/39654

   ### What changes were proposed in this pull request?
   This PR adds `ioe` to the warning log of `finalizeShuffleMerge`.
   
   ### Why are the changes needed?
   With `ioe` logged, user would have more clue as to the root cause.
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   Existing test suite.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on pull request #39654: [SHUFFLE][MINOR] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1397097053

   cc @mridulm 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
tedyu commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081954269


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   @mridulm 
   If my understanding is correct, the logger would show the last parameter if it is an exception.
   See the following code from common/network-common/src/main/java/org/apache/spark/network/client/TransportClient.java
   ```
           } catch (Throwable t) {
             logger.warn("Error in responding RPC callback", t);
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #39654: [SHUFFLE][MINOR] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081658172


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -848,13 +848,13 @@ public void registerExecutor(String appId, ExecutorShuffleInfo executorInfo) {
           = new TypeReference<Map<String, String>>(){};
         Map<String, String> metaMap = mapper.readValue(mergeDirInfo, typeRef);
         String mergeDir = metaMap.get(MERGE_DIR_KEY);
-        int attemptId = Integer.valueOf(
-          metaMap.getOrDefault(ATTEMPT_ID_KEY, String.valueOf(UNDEFINED_ATTEMPT_ID)));
         if (mergeDir == null) {
           throw new IllegalArgumentException(
             String.format("Failed to get the merge directory information from the " +
               "shuffleManagerMeta %s in executor registration message", shuffleManagerMeta));
         }
+        int attemptId = Integer.valueOf(
+          metaMap.getOrDefault(ATTEMPT_ID_KEY, String.valueOf(UNDEFINED_ATTEMPT_ID)));

Review Comment:
   This relocation is not needed for `Include IOException in warning log of finalzeShuffleMerge`. Please revert this relocation.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu closed pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu closed pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge
URL: https://github.com/apache/spark/pull/39654


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081684212


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   BTW, could you add the new error message to the PR description, @tedyu ?



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399319165

   @dongjoon-hyun 
   Do you think this PR is in mergeable state ?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen closed pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen closed pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge
URL: https://github.com/apache/spark/pull/39654


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399319090

   @srowen @mridulm 
   Tests passed.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399341276

   @dongjoon-hyun @srowen @mridulm 
   Thanks for reviewing this PR.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] mridulm commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
mridulm commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081952766


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   Also, the added parameter is not referenced in the format string.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] mridulm commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
mridulm commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081952766


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   The added parameter is not referenced in the format string.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399336797

   I'll leave this to the other committers, @tedyu .


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399271648

   Yeah looks fine, just rerun tests


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "srowen (via GitHub)" <gi...@apache.org>.
srowen commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399340205

   Merged to master


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
tedyu commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1397785390

   From https://github.com/tedyu/spark/actions/runs/3961295903/jobs/6786585143
   ```
   Finished test(pypy3): pyspark.sql.tests.test_utils (10s)
   Starting test(python3.9): pyspark.mllib.tests.test_algorithms (temp output: /__w/spark/spark/python/target/a23c65c0-281b-4044-8aab-2a99865b220b/python3.9__pyspark.mllib.tests.test_algorithms__oumchym_.log)
   Error: The operation was canceled.
   ```
   Not sure of the reason for the cancellation - maybe timeout ?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081772125


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   If you didn't hit this issue before, why do we need to pay attention on this code path?



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on a diff in pull request #39654: [SHUFFLE][MINOR] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
tedyu commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081661106


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -848,13 +848,13 @@ public void registerExecutor(String appId, ExecutorShuffleInfo executorInfo) {
           = new TypeReference<Map<String, String>>(){};
         Map<String, String> metaMap = mapper.readValue(mergeDirInfo, typeRef);
         String mergeDir = metaMap.get(MERGE_DIR_KEY);
-        int attemptId = Integer.valueOf(
-          metaMap.getOrDefault(ATTEMPT_ID_KEY, String.valueOf(UNDEFINED_ATTEMPT_ID)));
         if (mergeDir == null) {
           throw new IllegalArgumentException(
             String.format("Failed to get the merge directory information from the " +
               "shuffleManagerMeta %s in executor registration message", shuffleManagerMeta));
         }
+        int attemptId = Integer.valueOf(
+          metaMap.getOrDefault(ATTEMPT_ID_KEY, String.valueOf(UNDEFINED_ATTEMPT_ID)));

Review Comment:
   Reverted as requested.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] mridulm commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1083189284


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   Including stack trace in this warn message, is not helpful.
   It is not actionable by users



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
tedyu commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081692479


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   I did some search but haven't found sample error message (without IOE).
   Please advise how the new error message should be formulated.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1397832776

   That test shouldn't be related.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] mridulm commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399168240

   To clarify @tedyu, I am fine with including the exception message (ex.getMessage) in the warn message - not the entire stack trace - so there is value in the PR !


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu commented on PR #39654:
URL: https://github.com/apache/spark/pull/39654#issuecomment-1399242161

   Test failures were not related to the PR.
   https://github.com/tedyu/spark/actions/runs/3973317986/jobs/6811901738#step:9:23488
   ```
   Error: Exception in thread "streaming-job-executor-0" java.lang.Error: java.lang.InterruptedException
           at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1155)
           at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
           at java.lang.Thread.run(Thread.java:750)
   Caused by: java.lang.InterruptedException
           at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:998)
           at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1304)
           at scala.concurrent.impl.Promise$DefaultPromise.tryAwait(Promise.scala:242)
   ```


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] tedyu commented on a diff in pull request #39654: [MINOR][SHUFFLE] Include IOException in warning log of finalizeShuffleMerge

Posted by GitBox <gi...@apache.org>.
tedyu commented on code in PR #39654:
URL: https://github.com/apache/spark/pull/39654#discussion_r1081692479


##########
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java:
##########
@@ -815,7 +815,7 @@ public MergeStatuses finalizeShuffleMerge(FinalizeShuffleMerge msg) {
           } catch (IOException ioe) {
             logger.warn("{} attempt {} shuffle {} shuffleMerge {}: exception while " +
                 "finalizing shuffle partition {}", msg.appId, msg.appAttemptId, msg.shuffleId,
-                msg.shuffleMergeId, partition.reduceId);
+                msg.shuffleMergeId, partition.reduceId, ioe);

Review Comment:
   I did some search but haven't found sample error message (without IOE).
   Please advise how the new error message should be formulated.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org