You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celeborn.apache.org by GitBox <gi...@apache.org> on 2022/12/01 02:15:41 UTC

[GitHub] [incubator-celeborn] zhongqiangczq opened a new pull request, #1036: [CELEBORN-11] ShuffleClient supports MapPartition shuffle write:pushdataByteBuf

zhongqiangczq opened a new pull request, #1036:
URL: https://github.com/apache/incubator-celeborn/pull/1036

   pushdataByteBuf
   
   # [BUG]/[FEATURE] title
   
   ### What changes were proposed in this pull request?
   
   
   ### Why are the changes needed?
   
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   ### What are the items that need reviewer attention?
   
   
   ### Related issues.
   
   
   ### Related pull requests.
   
   
   ### How was this patch tested?
   
   
   /cc @related-reviewer
   
   /assign @main-reviewer
   


-- 
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-unsubscribe@celeborn.apache.org

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

[GitHub] [incubator-celeborn] zhongqiangczq commented on a diff in pull request #1036: [CELEBORN-11] ShuffleClient supports MapPartition shuffle write:pushdata

Posted by GitBox <gi...@apache.org>.
zhongqiangczq commented on code in PR #1036:
URL: https://github.com/apache/incubator-celeborn/pull/1036#discussion_r1042895704


##########
client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java:
##########
@@ -604,7 +608,6 @@ public void onSuccess(ByteBuffer response) {
                     .computeIfAbsent(shuffleId, (id) -> ConcurrentHashMap.newKeySet())
                     .add(mapKey);
               }
-              pushState.removeFuture(nextBatchId);

Review Comment:
   it's my fault. i will commit another pr to add it back 



-- 
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: issues-unsubscribe@celeborn.apache.org

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

[GitHub] [incubator-celeborn] waitinfuture commented on a diff in pull request #1036: [CELEBORN-11] ShuffleClient supports MapPartition shuffle write:pushdata

Posted by GitBox <gi...@apache.org>.
waitinfuture commented on code in PR #1036:
URL: https://github.com/apache/incubator-celeborn/pull/1036#discussion_r1042881207


##########
client/src/main/java/org/apache/celeborn/client/ShuffleClient.java:
##########
@@ -175,6 +177,20 @@ public abstract RssInputStream readPartition(
 
   public abstract void shutDown();
 
+  // Write data to a specific map partition, input data's type is Bytebuf.
+  // data's type is Bytebuf to avoid copy between application and netty
+  // closecallback will do some clean opertions like memory release.
+  public abstract int pushData(

Review Comment:
   Better to rename to pushDataToLocation



##########
client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java:
##########
@@ -604,7 +608,6 @@ public void onSuccess(ByteBuffer response) {
                     .computeIfAbsent(shuffleId, (id) -> ConcurrentHashMap.newKeySet())
                     .add(mapKey);
               }
-              pushState.removeFuture(nextBatchId);

Review Comment:
   why delete this line?



-- 
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: issues-unsubscribe@celeborn.apache.org

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

[GitHub] [incubator-celeborn] zhongqiangczq commented on a diff in pull request #1036: [CELEBORN-11] ShuffleClient supports MapPartition shuffle write:pushdata

Posted by GitBox <gi...@apache.org>.
zhongqiangczq commented on code in PR #1036:
URL: https://github.com/apache/incubator-celeborn/pull/1036#discussion_r1042903605


##########
client/src/main/java/org/apache/celeborn/client/ShuffleClient.java:
##########
@@ -175,6 +177,20 @@ public abstract RssInputStream readPartition(
 
   public abstract void shutDown();
 
+  // Write data to a specific map partition, input data's type is Bytebuf.
+  // data's type is Bytebuf to avoid copy between application and netty
+  // closecallback will do some clean opertions like memory release.
+  public abstract int pushData(

Review Comment:
   pushData is changed to pushDataToLocation



-- 
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: issues-unsubscribe@celeborn.apache.org

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

[GitHub] [incubator-celeborn] zhongqiangczq commented on a diff in pull request #1036: [CELEBORN-11] ShuffleClient supports MapPartition shuffle write:pushdata

Posted by GitBox <gi...@apache.org>.
zhongqiangczq commented on code in PR #1036:
URL: https://github.com/apache/incubator-celeborn/pull/1036#discussion_r1042895704


##########
client/src/main/java/org/apache/celeborn/client/ShuffleClientImpl.java:
##########
@@ -604,7 +608,6 @@ public void onSuccess(ByteBuffer response) {
                     .computeIfAbsent(shuffleId, (id) -> ConcurrentHashMap.newKeySet())
                     .add(mapKey);
               }
-              pushState.removeFuture(nextBatchId);

Review Comment:
   it's my fault. i will add it back 



-- 
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: issues-unsubscribe@celeborn.apache.org

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

[GitHub] [incubator-celeborn] waitinfuture merged pull request #1036: [CELEBORN-11] ShuffleClient supports MapPartition shuffle write:pushdata

Posted by GitBox <gi...@apache.org>.
waitinfuture merged PR #1036:
URL: https://github.com/apache/incubator-celeborn/pull/1036


-- 
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: issues-unsubscribe@celeborn.apache.org

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