[GitHub] [flink] azagrebin commented on a change in pull request #9975: [FLINK-14477][coordination] Implement promotion logic on TaskExecutor

[GitBox <gi...@apache.org>]

azagrebin commented on a change in pull request #9975: [FLINK-14477][coordination] Implement promotion logic on TaskExecutor 
URL: https://github.com/apache/flink/pull/9975#discussion_r345277386
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskExecutor.java
 ##########
 @@ -1371,10 +1369,7 @@ private void scheduleResultPartitionCleanup(JobID jobId) {
 		if (taskTerminationFutures != null) {
 			FutureUtils.waitForAll(taskTerminationFutures)
 				.thenRunAsync(() -> {
-					Collection<ResultPartitionID> partitionsForJob = CollectionUtil.project(
-						partitionTracker.stopTrackingPartitionsFor(jobId),
-						PartitionTrackerEntry::getResultPartitionId);
-					shuffleEnvironment.releasePartitionsLocally(partitionsForJob);
+					partitionTracker.stopTrackingAndReleaseJobPartitionsFor(jobId);
 
 Review comment:
   Did you consider moving even more things related only to partition tracking to `TaskExecutorPartitionTracker`?
   Like:
   `taskResultPartitionCleanupFuturesPerJob`
   `setupResultPartitionBookkeeping`
   `scheduleResultPartitionCleanup`
   
   TaskExecutor could accept a tracker factory to pass main thread executor. 
   
   in this case, maybe, not all methods from `PartitionTracker` are needed. It could be composition instead of inheritance.

----------------------------------------------------------------
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