[GitHub] [incubator-celeborn] AngersZhuuuu commented on a diff in pull request #1028: [CELEBORN-63] Increase push data return reason types such as CONGESTION ect

[GitBox]

AngersZhuuuu commented on code in PR #1028:
URL: 
https://github.com/apache/incubator-celeborn/pull/1028#discussion_r1035648982


##########
common/src/main/scala/org/apache/celeborn/common/util/Utils.scala:
##########
@@ -890,6 +890,12 @@ object Utils extends Logging {
         StatusCode.WORKER_IN_BLACKLIST
       case 28 =>
         StatusCode.UNKNOWN_WORKER
+      case 30 =>
+        StatusCode.SUCCESS_FREE

Review Comment:
   PUSH_SUCCESS



##########
common/src/main/scala/org/apache/celeborn/common/util/Utils.scala:
##########
@@ -890,6 +890,12 @@ object Utils extends Logging {
         StatusCode.WORKER_IN_BLACKLIST
       case 28 =>
         StatusCode.UNKNOWN_WORKER
+      case 30 =>
+        StatusCode.SUCCESS_FREE
+      case 31 =>
+        StatusCode.SUCCESS_MASTER_CONGESTED

Review Comment:
   ditto



##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala:
##########
@@ -262,7 +262,7 @@ class PushDataHandler extends BaseMessageHandler with 
Logging {
         }
       })
     } else {
-      wrappedCallback.onSuccess(ByteBuffer.wrap(Array[Byte]()))
+      
wrappedCallback.onSuccess(ByteBuffer.wrap(Array[Byte](StatusCode.SUCCESS_FREE.getValue)))

Review Comment:
   We should also change this in `pushMergedData`



##########
client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java:
##########
@@ -591,6 +591,7 @@ public int pushOrMergeData(
             @Override
             public void onSuccess(ByteBuffer response) {
               pushState.inFlightBatches.remove(nextBatchId);
+              // TODO Need to handle CONGESTED & FREE statuses

Review Comment:
   Add comment about why not handle PUSH_SUCCES, also add in pushMergedData



##########
common/src/main/scala/org/apache/celeborn/common/util/Utils.scala:
##########
@@ -890,6 +890,12 @@ object Utils extends Logging {
         StatusCode.WORKER_IN_BLACKLIST
       case 28 =>
         StatusCode.UNKNOWN_WORKER
+      case 30 =>
+        StatusCode.SUCCESS_FREE
+      case 31 =>
+        StatusCode.SUCCESS_MASTER_CONGESTED
+      case 32 =>
+        StatusCode.SUCCESS_SLAVE_CONGESTED

Review Comment:
   ditto



-- 
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: dev-unsubscr...@celeborn.apache.org

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