zhijiangW commented on a change in pull request #7911: [FLINK-11082][network] 
Fix the logic of getting backlog in sub partition
URL: https://github.com/apache/flink/pull/7911#discussion_r264503784
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/SpillableSubpartitionView.java
 ##########
 @@ -156,7 +156,8 @@ public BufferAndBacklog getNextBuffer() throws 
IOException, InterruptedException
                                checkState(nextBuffer.isFinished(),
                                        "We can only read from 
SpillableSubpartition after it was finished");
 
-                               newBacklog = 
parent.decreaseBuffersInBacklogUnsafe(nextBuffer.isBuffer());
+                               
parent.decreaseBuffersInBacklog(nextBuffer.isBuffer());
+                               newBacklog = parent.getBuffersInBacklog();
 
 Review comment:
   1. The current backlog is based on `flushTriggered` or `isFinished` status 
after changing, if we want to return backlog during `decreaseBuffersInBacklog`, 
we might need to pass these info into `decreaseBuffersInBacklog(boolean 
flushTriggered | isFinished)`. In order not to dirty this method, I split it 
into a separate method. Maybe it does not seem so bad if passing parameter into 
`decreaseBuffersInBacklog `. So we could still merge these two into one method 
if you prefer.
   
   2. Yes, you are right. Here we could get backlog in unsafe way, no need to 
add the synchronized overhead.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to