zhijiangW commented on a change in pull request #7186: [FLINK-10941] Keep slots
which contain unconsumed result partitions
URL: https://github.com/apache/flink/pull/7186#discussion_r238983295
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/PartitionRequestQueue.java
##########
@@ -134,10 +138,17 @@ public void cancel(InputChannelID receiverId) {
ctx.pipeline().fireUserEventTriggered(receiverId);
}
- public void close() {
+ public void close() throws IOException {
if (ctx != null) {
ctx.channel().close();
}
+
+ LOG.info("Close all {} readers pending for close.",
readersToClose.size());
Review comment:
Sorry for late response because of a little busy these days.
I agree with your above comments. So whether the task executor can be
released is based on whether there are active channels in this executor. The
task executor can only exit after all the tcp connections are closed gracefully.
In theory as long as the downstream received all the data from the network,
then the upstream side can be released normally, no need to wait all the
received data are processed completely by downstream side. But we have on
existing ack mechanism to notify upstream side of this, so it is easy to rely
on close request currently. Based on downstream's consumption to release
upstream's resource, it may get extra benefits in failover scenarios in future
for persistent output files in upstream side, because the upstream do not need
to restart during consumption exception in downstream side.
But I just a little wonder it might bring potential effects in future via
close request. For example, if there are 10 downstream tasks reuse the same tcp
connection, and 9 tasks are finished earlier and only one tail task delay long
to finish, then all the 10 partition views must be released together until the
last downstream task finished. Although it might be no bad effects for delay
releasing partition views currently.
I would continue reviewing other parts of this PR and it may take some days
on my side. :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
