azagrebin 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_r334582892
##########
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:
One more idea, it looks like the legacy fields can be mapped to the new ones:
`taskManagerHeapSizeMB` -> `taskHeapSize`
`taskManagerDirectMemoryLimitMB` -> `offHeapManagedMemorySize` +
`shuffleMemSize`(can be e.g. zero for legacy)
others could stay zero
Then `ContaineredTaskManagerParameters` could rely on
`TaskExecutorResourceSpec` for new and legacy modes.
`ENABLE_FLIP_49_CONFIG` could be resolved in `create` method, also
deduplicating code in yarn/mesos,
especially if we cache the whole `ContaineredTaskManagerParameters` in
`YarnResourceManager` as a field instead of just `TaskExecutorResourceSpec`
(should be ok as resource memory seems to be always
`defaultTaskManagerMemoryMB` if I am not confusing it).
Generation of JVM args could be then deduplicated and
`TaskExecutorResourceUtils` is used everywhere, especially if we add
jvmDirectSize/jvmMetaspaceSize there only if they are not zero like legacy mode
already does.
Dynamic parameters should not hurt in legacy mode either as they should just
stay unused.
This would probably also allow to extend only a bit
`testGetTaskManagerShellCommand` to cover additional non-zero args instead of
splitting it.
Eventually that should simplify the code split.
----------------------------------------------------------------
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
