zhijiangW commented on a change in pull request #8789: [FLINK-12890] Add
partition lifecycle related Shuffle API
URL: https://github.com/apache/flink/pull/8789#discussion_r296625464
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/shuffle/ShuffleMaster.java
##########
@@ -41,4 +44,17 @@
CompletableFuture<T> registerPartitionWithProducer(
PartitionDescriptor partitionDescriptor,
ProducerDescriptor producerDescriptor);
+
+ /**
+ * Release any external resources occupied by the given partition.
+ *
+ * <p>This call triggers release of any resources which are occupied by
the given partition in the external systems
+ * outside of the producer executor. This is mostly relevant for the
batch jobs and blocking result partitions
+ * for which {@link
ResultPartitionDeploymentDescriptor#isReleasedOnConsumption()} returns {@code
false}.
+ * The producer local resources are managed by {@link
ShuffleDescriptor#hasLocalResources()} and
+ * {@link ShuffleEnvironment#releasePartitions(Collection)}.
+ *
+ * @param shuffleDescriptor shuffle descriptor of the result partition
to release externally.
+ */
+ void releasePartitionExternally(T shuffleDescriptor);
Review comment:
Yes, my previous suggestion was defined the methods as
`ShuffleMaster#releasePartitions` and `ShuffleEnvironment#releasePartitions`.
In default netty implementation we actually do not use this path for releasing
partitions.
For other extending new implementations(no matter external or other
internal), users could decide whether to use this path or not. If use, it
should adjust to let JM call `ShuffleMaster#releasePartition`, not communicate
directly with TM as now.
I am not strongly insisting on this suggestion, just pointed out some
personal concerns from first impression. And I think this summary-up could help
deeply understand the full stack via discussion. :)
Also agree with your issue of `PD#releaseOnConsumption`, the partition
release strategy could be abstracted to pass into `PartitionDescriptor` and
`ShuffleMaster` could check whether the current implementation supports it.
----------------------------------------------------------------
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
