tillrohrmann commented on a change in pull request #10362:
[FLINK-14792][coordination] Implement TE cluster partition release
URL: https://github.com/apache/flink/pull/10362#discussion_r394500308
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/taskexecutor/TaskExecutorPartitionLifecycleTest.java
##########
@@ -263,6 +264,24 @@ public void testPartitionPromotion() throws Exception {
);
}
+ @Test
+ public void testClusterPartitionRelease() throws Exception {
+ testPartitionRelease(
+ partitionTracker -> {
+ final
CompletableFuture<Collection<IntermediateDataSetID>> releasePartitionsFuture =
new CompletableFuture<>();
+
partitionTracker.setReleaseClusterPartitionsConsumer(releasePartitionsFuture::complete);
+ return releasePartitionsFuture;
+ },
+ (jobId, partitionId, taskExecutor, taskExecutorGateway,
releasePartitionsFuture) -> {
+ final IntermediateDataSetID dataSetId = new
IntermediateDataSetID();
+
+
taskExecutorGateway.releaseClusterPartitions(Collections.singleton(dataSetId));
+
+ assertThat(releasePartitionsFuture.get(),
hasItems(dataSetId));
+ }
+ );
+ }
Review comment:
I still think that this test is a bit overblown. I think it would be good
enough to test that when calling
`TaskExecutorPartitionTracker#stopTrackingAndReleaseClusterPartitions` that we
release the correct partitions on the `ShuffleEnvironment`. What this code
currently tests is that `TaskExecutorGateway#releaseClusterPartitions` calls
`TaskExecutorPartitionTracker#stopTrackingAndReleaseClusterPartitions`. Not
sure whether starting the whole machinery for such a trivial is worth 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
