flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] zhijiangW commented on a change in pull request #7911: [FLINK-11082][network] Fix the logic of getting backlog in sub partition
Date Fri, 15 Mar 2019 04:16:49 GMT
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_r265845525
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/SpilledSubpartitionView.java
 ##########
 @@ -147,7 +147,8 @@ public BufferAndBacklog getNextBuffer() throws IOException, InterruptedException
 			return null;
 		}
 
-		int newBacklog = parent.decreaseBuffersInBacklog(current.isBuffer());
+		parent.decreaseBuffersInBacklog(current.isBuffer());
+		int newBacklog = parent.getBuffersInBacklog();
 
 Review comment:
   That is a good question that should be concerned. I considered it again and thought the
synchronized is not needed for getting backlog because this value would be final consistent
between sender and receiver.
   
   E.g. if the current backlog is 4 after decreasing, the previous behavior would report 4
strictly. The new behavior might report 4 or more than 4 if increasing backlog again before
getting. But the result is still correct if reporting 5 because it actually exists. The difference
is we report this increase in advance, and the previous behavior would reflect this increase
in the next report. The early report might get some extra benefits because the receiver could
prepare more credits for it.
   
   It could have two options:
   1. Uniform all the ways of getting backlog outside of synchronized.
   2. Integrate return backlog in `decreaseBuffersInBacklog` as you mentioned, and the form
seems not so bad if adding boolean parameter in `decreaseBuffersInBacklog`. The backlog is
strictly returned in this way.
   
   Which option do you prefer?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message