xintongsong commented on a change in pull request #11445: [FLINK-16615]
Introduce data structures and utilities to calculate Job Manager memory
components
URL: https://github.com/apache/flink/pull/11445#discussion_r394816211
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/TaskExecutorProcessUtils.java
##########
@@ -222,26 +241,22 @@ private static TaskExecutorProcessSpec
deriveProcessSpecWithTotalFlinkMemory(fin
// derive jvm metaspace and overhead
- final JvmMetaspaceAndOverhead jvmMetaspaceAndOverhead =
deriveJvmMetaspaceAndOverheadFromTotalFlinkMemory(config, totalFlinkMemorySize);
+ final JvmMetaspaceAndOverhead jvmMetaspaceAndOverhead =
deriveJvmMetaspaceAndOverheadFromTotalFlinkMemory(
+ config,
+ totalFlinkMemorySize,
+ TM_JVM_METASPACE_AND_OVERHEAD_OPTIONS);
return createTaskExecutorProcessSpec(config,
flinkInternalMemory, jvmMetaspaceAndOverhead);
}
private static TaskExecutorProcessSpec
deriveProcessSpecWithTotalProcessMemory(final Configuration config) {
// derive total flink memory from explicitly configured total
process memory size
- final MemorySize totalProcessMemorySize =
getTotalProcessMemorySize(config);
- final MemorySize jvmMetaspaceSize = getJvmMetaspaceSize(config);
- final MemorySize jvmOverheadSize =
deriveJvmOverheadWithFraction(config, totalProcessMemorySize);
- final JvmMetaspaceAndOverhead jvmMetaspaceAndOverhead = new
JvmMetaspaceAndOverhead(jvmMetaspaceSize, jvmOverheadSize);
-
- if
(jvmMetaspaceAndOverhead.getTotalJvmMetaspaceAndOverheadSize().getBytes() >
totalProcessMemorySize.getBytes()) {
- throw new IllegalConfigurationException(
- "Sum of configured JVM Metaspace (" +
jvmMetaspaceAndOverhead.metaspace.toHumanReadableString()
- + ") and JVM Overhead (" +
jvmMetaspaceAndOverhead.overhead.toHumanReadableString()
- + ") exceed configured Total Process Memory ("
+ totalProcessMemorySize.toHumanReadableString() + ").");
- }
- final MemorySize totalFlinkMemorySize =
totalProcessMemorySize.subtract(jvmMetaspaceAndOverhead.getTotalJvmMetaspaceAndOverheadSize());
+ final JvmMetaspaceAndOverhead jvmMetaspaceAndOverhead =
+
deriveJvmMetaspaceAndOverheadWithTotalProcessMemory(config,
TM_JVM_METASPACE_AND_OVERHEAD_OPTIONS);
+ MemorySize totalProcessMemorySize =
getMemorySizeFromConfig(config, TaskManagerOptions.TOTAL_PROCESS_MEMORY);
Review comment:
The total process memory size is fetched twice, inside and after
`deriveJvmMetaspaceAndOverheadWithTotalProcessMemory`.
It might be better to fetch it once, and just pass it as an argument into
`deriveJvmMetaspaceAndOverheadWithTotalProcessMemory`, to avoid the risk of
inconsistency.
----------------------------------------------------------------
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
