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_r334736805
##########
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:
I'm not sure I understand this comment completely.
First of all, as mentioned above, the old parameters cannot be mapped to
`TaskExecutorResourceSpec` directly.
I'd like to keep the two configuration logics separately for the following
reasons:
- This PR means to introduce no behavior changes to the legacy mode, which
is hard to guarantee if we try to map the old fields to the new ones. Since the
legacy code path is quite complex and not friendly to maintainability (exactly
one of the motivations for FLIP-49), and will be removed soon, I'd rather not
to spend too much effort on deduplication of the two code paths with the risk
of breaking the original behavior of the legacy mode.
- The code path of the new mode, in my opinion, is clear and self contained.
I'm in favor of not messing it by inserting the legacy logics in between. That
would also introduce more workload for later legacy clean-ups.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
