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_r335057546
##########
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 understand the motivation of having as less as possible separated code
paths, for exposing the new codes for testability early.
My key point is that, this cannot be easily done.
- The old fields cannot be mapped to the new fields.
`onHeapManagedMemorySize` does not match `taskManagerHeapSizeMB` neither. The
legacy filed `taskManagerHeapSizeMB` has a "heap" in its name, but is actually
not describing java heap memory. I don't think any of the filed in
`TaskExecutorResourceSpec` matches it.
- FLIP-49 is designed to change the way of memory configuration. We can
foresee that not all of the IT/E2E cases can work as it is. It doesn't make
sense to try to expose the new configuration logics to the ETE tests at this
moment.
- My plan was to first get the new feature completed, then switching IT/ETE
test cases to the new configuration (because we no longer need them to guard
the old logic). That's why I put the work item "fix / update / remove test
cases for legacy mode" in the last step "clean-up of legacy mode" of FLIP-49.
----------------------------------------------------------------
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
