zentol commented on a change in pull request #18749:
URL: https://github.com/apache/flink/pull/18749#discussion_r806855733



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/cleanup/DispatcherResourceCleanerFactory.java
##########
@@ -101,13 +101,13 @@ public ResourceCleaner createLocalResourceCleaner(
     @Override
     public ResourceCleaner createGlobalResourceCleaner(
             ComponentMainThreadExecutor mainThreadExecutor) {
-
         return DefaultResourceCleaner.forGloballyCleanableResources(
                         mainThreadExecutor, cleanupExecutor, retryStrategy)
-                .withPrioritizedCleanup(jobManagerRunnerRegistry)
+                
.withPrioritizedCleanup(ofLocalResource(jobManagerRunnerRegistry))
                 .withRegularCleanup(jobGraphWriter)

Review comment:
       I don't understand why we need to replicate things here.
   
   Shouldn't every local resources always be cleaned up on a global cleanup? 
Why can we not, in the case where currently we'd use the global cleaner, first 
call the local cleaner and then run the global one?
   
   Is there any use-case where a local resource should _not_ be cleaned up on a 
global terminal state?
   Is there any use-case where a local resource should be cleaned _after_ a 
global resource? (or vice-versa, where a global resource should be cleaned up 
_before_ a local resource).
   
   These question are primarily based on my assumption that the only benefit of 
duplicating everything is that it allows us to cleanup things in a specific 
order that local cleanup -> global cleanup wouldn't provide.




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

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to