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 `finished` status
after changing, if we want to return backlog during `decreaseBuffersInBacklog`,
we might need to pass these info into `decreaseBuffersInBacklog(boolean
`flushTriggered` | `finished`)`. 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:
[email protected]
With regards,
Apache Git Services
