zhijiangW commented on a change in pull request #7185: [FLINK-10884] 
[yarn/mesos]  adjust  container memory param  to set a safe margin from offheap 
memory
URL: https://github.com/apache/flink/pull/7185#discussion_r237743628
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/ContaineredTaskManagerParameters.java
 ##########
 @@ -158,8 +158,10 @@ public static ContaineredTaskManagerParameters create(
 
                // (2) split the remaining Java memory between heap and off-heap
                final long heapSizeMB = 
TaskManagerServices.calculateHeapSizeMB(containerMemoryMB - cutoffMB, config);
-               // use the cut-off memory for off-heap (that was its intention)
-               final long offHeapSizeMB = containerMemoryMB - heapSizeMB;
+               // (3) try to compute the offHeapMemory from a safe margin
+               final long restMemoryMB = containerMemoryMB - heapSizeMB;
+               final long offHeapCutoffMemory = 
calculateOffHeapCutoffMB(config, restMemoryMB);
 
 Review comment:
   Currently we already have a `containerized.heap-cutoff-ratio` for reserving 
some memories for other usages. And the `heapSizeMB` is calculated based on 
`containerMemoryMB - cutoffMB`, so the `heapSizeMB+ offHeapSizeMB` should be 
`containerMemoryMB-cutoffMB`.
   
   You further extend the `cutoff-ratio` to 
`containerized.offheap-cutoff-ratio`. I think there are two options:
   
   1. Adjust the existing `containerized.heap-cutoff-ratio` to 
`containerized.cutoff-ratio` which means reserving some physical memories used 
for both heap and off-heap.
   
   2. Separate into two different parameters as you provide. I am not sure 
whether it can get extra benefits compared with first option. But it may make 
the things a little complicated, because the memory can be further divided into 
heap, direct, native (used by rockdb state backend). The direct and native 
memories can be both regarded as off-heap in general speaking. If to do so, do 
we also need `containerized.offheap-cutoff-min` matched with existing 
`containerized.heap-cutoff-min`?
   
   BTW, I think you can increase the current `containerized.heap-cutoff-ratio` 
and  `containerized.heap-cutoff-min` to avoid container killed because of 
exceeding memories. :)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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

Reply via email to