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_r334591170
##########
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:
Do we need to duplicate the whole `TaskManagerServicesConfiguration`
creation?
Could we just create a function to resolve `TaskExecutorResourceSpec` and
put it as a constructor argument to `TaskManagerServicesConfiguration`?
I would be again in favour of trying to map the old parameters to the new
`TaskExecutorResourceSpec` instead of keeping it null. The factory function for
`TaskExecutorResourceSpec` could resolve legacy and new modes assigning
correctly: `onHeapManagedMemorySize`, `offHeapManagedMemorySize` and
`shuffleMemSize`.
We could move the legacy `shuffleMemSize` calculation from
`NettyShuffleEnvironmentConfiguration. calculateNumberOfNetworkBuffers` (the
oldest one is just numBuffers x page) and keep only `networkMemorySizeByte /
pageSizeByte` there.
For the memory manager (next commit, could be merged with this), again
calculation of memory size for the configured legacy type can be factored out
from `TaskManagerServices.createMemoryManager` and go to the respective
onHeapManagedMemorySize/offHeapManagedMemorySize, other type stays zero.
Then the only new path in `NettyShuffleEnvironmentConfiguration.
calculateNumberOfNetworkBuffers` and `TaskManagerServices.createMemoryManager`
should work for all cases which reduces 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:
[email protected]
With regards,
Apache Git Services
