xintongsong commented on a change in pull request #9801: [FLINK-13983][runtime] 
Launch task executor with new memory calculation logics
URL: https://github.com/apache/flink/pull/9801#discussion_r329350267
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/ContaineredTaskManagerParameters.java
 ##########
 @@ -48,13 +48,17 @@
        /** Environment variables to add to the Java process. */
        private final HashMap<String, String> taskManagerEnv;
 
+       private final TaskExecutorResourceSpec taskExecutorResourceSpec;
+
        public ContaineredTaskManagerParameters(
+                       TaskExecutorResourceSpec taskExecutorResourceSpec,
 
 Review comment:
   I'm not sure about that. Having different constructors means that 
`ContaineredTaskManagerParameters` needs to understand the difference between 
legacy and flip49 modes, and initialize final class members that are not valid 
in the current mode with `null` values. I think we should have as less as 
possible separated code paths, thus I'm in favor of keep it the current way.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to