You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "vastian180 (via GitHub)" <gi...@apache.org> on 2025/05/05 06:04:39 UTC

[PR] [CELEBORN-1487][FOLLOWUP] Fix updateProduceBytes [celeborn]

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

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     - Make sure the PR title start w/ a JIRA ticket, e.g. '[CELEBORN-XXXX] Your PR title ...'.
     - Be sure to keep the PR description updated to reflect all changes.
     - Please write your PR title to summarize what this PR proposes.
     - If possible, provide a concise example to reproduce the issue for a faster review.
   -->
   
   ### What changes were proposed in this pull request?
   In the `updateProduceBytes` method, `userBufferInfo.bufferStatusHub` and `workerBufferStatusHub` add the same node. This results in the lastNode's numBytes being repeatedly accumulated in the `_deque` of both hubs if updateProduceBytes is called multiple times within one second, since the `lastNode` is the same object.
   
   This will cause the numBytes of `nodeToSeparate` to become negative when the `removeExpiredNodes` method is called, specifically at the line `nodeToSeparate.separateNode(removed.getRight());` , which will make avgBytesPerSec return 0.
   
   As shown in the figure below, this value remains at 0 for a long period of time in the Dashborad.
   
   ![image](https://github.com/user-attachments/assets/4d85d4d1-45f9-4ba0-8cc7-ad2ceb504abe)
   
   
   ### Why are the changes needed?
   Fix updateProduceBytes method.
   
   
   ### Does this PR introduce _any_ user-facing change?
   NO
   
   
   ### How was this patch tested?
   UT
   AND Dashborad：
   ![image](https://github.com/user-attachments/assets/08212f13-39d3-42a8-b178-8ec8be6d0822)
   
   
   


-- 
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-1487][FOLLOWUP] Fix updateProduceBytes [celeborn]

Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on PR #3240:
URL: https://github.com/apache/celeborn/pull/3240#issuecomment-2868896114

   Thanks merge to main(v0.6.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-1487][FOLLOWUP] Fix updateProduceBytes [celeborn]

Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong closed pull request #3240: [CELEBORN-1487][FOLLOWUP] Fix updateProduceBytes
URL: https://github.com/apache/celeborn/pull/3240


-- 
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-1487][FOLLOWUP] Fix updateProduceBytes [celeborn]

Posted by "vastian180 (via GitHub)" <gi...@apache.org>.
vastian180 commented on PR #3240:
URL: https://github.com/apache/celeborn/pull/3240#issuecomment-2853464677

   ping @onebox-li ，I would like to ask for your advice~
   
   I noticed that in this PR [[CELEBORN-1083] Refine TimeSlidingHub code](https://github.com/apache/celeborn/pull/2033/files#diff-c85b7ab1807b2bc925b8e56f73fd810fc8e4ce6a7f4f13acfb191c53e20054c4) , the clone operation on newNode was removed when calling _deque.add. What was the consideration behind this change?
   
   It seems that this could cause issues in scenarios like the following code: that is, multiple BufferStatusHubs add the same node.
   https://github.com/apache/celeborn/blob/main/worker/src/main/java/org/apache/celeborn/service/deploy/worker/congestcontrol/UserCongestionControlContext.java#L70
   
   Therefore, I submitted this PR to fix this usage issue.
   


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