You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "codenohup (via GitHub)" <gi...@apache.org> on 2025/02/17 10:00:54 UTC

[PR] [CELEBORN-1866][Flink] Fix CelebornChannelBufferReader request more buffers than needed [celeborn]

codenohup opened a new pull request, #3102:
URL: https://github.com/apache/celeborn/pull/3102

   
   ### What changes were proposed in this pull request?
   Fix CelebornChannelBufferReader request more buffers than needed.
   For more details, please refer to https://issues.apache.org/jira/projects/CELEBORN/issues/CELEBORN-1866?filter=allopenissues.
   
   
   ### Why are the changes needed?
   It may cause flink job hang.
   
   
   ### Does this PR introduce _any_ user-facing change?
   no.
   
   
   ### How was this patch tested?
   We have a job that can reproduce this case. After the fix, it can be completed stably.
   


-- 
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

Re: [PR] [CELEBORN-1866][Flink] Fix CelebornChannelBufferReader request more buffers than needed [celeborn]

Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #3102:
URL: https://github.com/apache/celeborn/pull/3102#discussion_r1957974178


##########
client-flink/flink-1.20/src/main/java/org/apache/celeborn/plugin/flink/tiered/CelebornChannelBufferReader.java:
##########
@@ -195,7 +203,9 @@ public void notifyAvailableCredits(int numCredits) {
               .setStreamId(bufferStream.getStreamId())
               .setCredit(numCredits)
               .build());
-      bufferManager.decreaseRequiredCredits(numCredits);
+      if (needDecreaseRequiredCredits) {
+        bufferManager.decreaseRequiredCredits(numCredits);

Review Comment:
   Could this move into `notifyAvailableCredits(int numCredits)`?



-- 
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

Re: [PR] [CELEBORN-1866][Flink] Fix CelebornChannelBufferReader request more buffers than needed [celeborn]

Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #3102:
URL: https://github.com/apache/celeborn/pull/3102#discussion_r1958082119


##########
client-flink/flink-1.20/src/main/java/org/apache/celeborn/plugin/flink/tiered/CelebornChannelBufferReader.java:
##########
@@ -124,7 +124,9 @@ public CelebornChannelBufferReader(
   public void setup(TieredStorageMemoryManager memoryManager) {
     this.bufferManager = new CelebornChannelBufferManager(memoryManager, this);
     if (numBackLog > 0) {
-      notifyAvailableCredits(bufferManager.requestBuffers(numBackLog));
+      int numRequestedBuffers = bufferManager.requestBuffers(numBackLog);

Review Comment:
   Does this need to check whether `numRequestedBuffers` is greater than 0?



-- 
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

Re: [PR] [CELEBORN-1866][Flink] Fix CelebornChannelBufferReader request more buffers than needed [celeborn]

Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa closed pull request #3102: [CELEBORN-1866][Flink] Fix CelebornChannelBufferReader request more buffers than needed
URL: https://github.com/apache/celeborn/pull/3102


-- 
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