advancedxy commented on code in PR #1718:
URL: 
https://github.com/apache/incubator-uniffle/pull/1718#discussion_r1626084627


##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -487,6 +487,8 @@ public void releaseReadMemory(long size) {
 
   // flush the buffer with required map which is <appId -> shuffleId>

Review Comment:
   Let's update this comment to reflect the new change.



##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -487,6 +487,8 @@ public void releaseReadMemory(long size) {
 
   // flush the buffer with required map which is <appId -> shuffleId>
   public synchronized void flush(Map<String, Set<Integer>> requiredFlush) {

Review Comment:
   Not related to this PR. But I think this flush method should be private 
instead so that we can change it freely.



##########
server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBufferManager.java:
##########
@@ -499,13 +501,19 @@ public synchronized void flush(Map<String, Set<Integer>> 
requiredFlush) {
             for (Map.Entry<Range<Integer>, ShuffleBuffer> rangeEntry :
                 shuffleIdToBuffers.getValue().asMapOfRanges().entrySet()) {
               Range<Integer> range = rangeEntry.getKey();
+              ShuffleBuffer shuffleBuffer = rangeEntry.getValue();
+              pickedFlushSize += shuffleBuffer.getSize();
               flushBuffer(
-                  rangeEntry.getValue(),
+                  shuffleBuffer,
                   appId,
                   shuffleId,
                   range.lowerEndpoint(),
                   range.upperEndpoint(),
                   isHugePartition(appId, shuffleId, range.lowerEndpoint()));
+              if (pickedFlushSize > expectedFlushSize) {
+                LOG.info("Finally flush {} bytes", pickedFlushSize);

Review Comment:
   Nit: `Already picked enough buffers to flush {} bytes`



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to