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_r334834786
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServicesConfiguration.java
##########
@@ -259,6 +271,30 @@ public static TaskManagerServicesConfiguration
fromConfiguration(
final RetryingRegistrationConfiguration
retryingRegistrationConfiguration =
RetryingRegistrationConfiguration.fromConfiguration(configuration);
+ if
(configuration.getBoolean(TaskManagerOptions.ENABLE_FLIP_49_CONFIG)) {
+ final TaskExecutorResourceSpec taskExecutorResourceSpec
= TaskExecutorResourceUtils.resourceSpecFromConfig(configuration);
Review comment:
The idea was that if we had less code path splits now then we would have
less work later when we remove/deprecate the old approach, ideally soon :). The
reason is that the CI would already check that the most of the code already
works for both old/new mode and we will give it exposure in E2E tests. This way
we automatically guarantee that the new code is better integrated into the
overall code which currently relies on legacy approach. Also, if we have the
feature switch only in two places (creating `TaskExecutorResourceSpec` for
container parameters and in TM config) the difference between old/new modes
will be also more observable and easy to remove.
True, if `taskHeapSize` is probably worse choice for `taskManagerHeapSizeMB`
than e.g. `onHeapManagedMemorySize`. If we keep all heap sizes zero except one
for legacy mode then it seems new code should work for legacy already now
(modulo do not add zero sizes to JVM args).
What are any other big obstacles for this approach?
----------------------------------------------------------------
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
