You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "Marvel-Gu (via GitHub)" <gi...@apache.org> on 2025/03/05 23:37:19 UTC

[PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Marvel-Gu opened a new pull request, #3458:
URL: https://github.com/apache/amoro/pull/3458

   ## Why are the changes needed?
   
   Close #3451.
   
   ## Brief change log
   <!--
   Clearly describe the changes made in modules, classes, methods, etc.
   -->
   In ams KubernetesOptimizerContainer, limit the size of the JVM heap memory to 80% of the pod's memory limit.
   
   ## How was this patch tested?
   
   - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
   
   - [ ] Add screenshots for manual tests if appropriate
   
   - [x] Run test locally before making a pull request
   
   ## Documentation
   
   - Does this pull request introduce a new feature? no
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "Marvel-Gu (via GitHub)" <gi...@apache.org>.
Marvel-Gu commented on code in PR #3458:
URL: https://github.com/apache/amoro/pull/3458#discussion_r1997401834


##########
charts/amoro/values.yaml:
##########
@@ -278,6 +278,7 @@ optimizer:
       #kube-config-path: "~/.kube/config"
       image: "apache/amoro:latest"
       pullPolicy: "IfNotPresent"
+      jvm.heap.ratio: "0.8"
   extra: []

Review Comment:
   Yes, putting it in extra is a better approach, I will modify it



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "czy006 (via GitHub)" <gi...@apache.org>.
czy006 commented on PR #3458:
URL: https://github.com/apache/amoro/pull/3458#issuecomment-2720945643

   > The default 80% configuration looks good to me. Maybe we can merge this PR now or do we have some other thing to validate before that? @Marvel-Gu @czy006
   
   This is indeed a black magic in the container. We can merge this PR now, and we may have a better way to solve it in the future.
   
   > #3451
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "Marvel-Gu (via GitHub)" <gi...@apache.org>.
Marvel-Gu commented on PR #3458:
URL: https://github.com/apache/amoro/pull/3458#issuecomment-2702347221

   I didn't format the code😂, I'll pay attention next time.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "Marvel-Gu (via GitHub)" <gi...@apache.org>.
Marvel-Gu commented on PR #3458:
URL: https://github.com/apache/amoro/pull/3458#issuecomment-2721041236

   > > The default 80% configuration looks good to me. Maybe we can merge this PR now or do we have some other thing to validate before that? @Marvel-Gu @czy006
   > 
   > This is indeed a black magic in the container. We can merge this PR now, and we may have a better way to solve it in the future.
   
   This is the simplest and most effective solution I could think of for now. How do you think it could be improved?


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on PR #3458:
URL: https://github.com/apache/amoro/pull/3458#issuecomment-2720129743

   The default 80% configuration looks good to me. Maybe we can merge this PR now or do we have some other thing to validate before that? @Marvel-Gu @czy006 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "czy006 (via GitHub)" <gi...@apache.org>.
czy006 commented on code in PR #3458:
URL: https://github.com/apache/amoro/pull/3458#discussion_r1982611322


##########
amoro-ams/src/main/java/org/apache/amoro/server/manager/KubernetesOptimizerContainer.java:
##########
@@ -148,18 +148,23 @@ public Map<String, Object> generatePodStartArgs(
       Resource resource, Map<String, String> groupProperties) {
     long memoryPerThread;
     long memory;
+    long jvmHeapMemory;
 
     if (resource.getMemoryMb() > 0) {
       memory = resource.getMemoryMb();
     } else {
       memoryPerThread = Long.parseLong(checkAndGetProperty(groupProperties, MEMORY_PROPERTY));
       memory = memoryPerThread * resource.getThreadCount();
     }
+
+    // reserving 20% of total pod memory for JVM non-heap memory
+    jvmHeapMemory = (long) (memory * 0.8);

Review Comment:
   If we do not use off-heap memory in the code, I think it can be set to 0.9, which will not cause the container to be killed. If it is still frequently killed by OOM, we may need to check whether there is a memory leak in the code or the instantaneous Heap usage exceeds the container limit. We may be able to provide an external parameter for users to set, WDYT?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "Marvel-Gu (via GitHub)" <gi...@apache.org>.
Marvel-Gu commented on code in PR #3458:
URL: https://github.com/apache/amoro/pull/3458#discussion_r1983399104


##########
amoro-ams/src/main/java/org/apache/amoro/server/manager/KubernetesOptimizerContainer.java:
##########
@@ -148,18 +148,23 @@ public Map<String, Object> generatePodStartArgs(
       Resource resource, Map<String, String> groupProperties) {
     long memoryPerThread;
     long memory;
+    long jvmHeapMemory;
 
     if (resource.getMemoryMb() > 0) {
       memory = resource.getMemoryMb();
     } else {
       memoryPerThread = Long.parseLong(checkAndGetProperty(groupProperties, MEMORY_PROPERTY));
       memory = memoryPerThread * resource.getThreadCount();
     }
+
+    // reserving 20% of total pod memory for JVM non-heap memory
+    jvmHeapMemory = (long) (memory * 0.8);

Review Comment:
   Sounds great, I'll work on it.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong merged PR #3458:
URL: https://github.com/apache/amoro/pull/3458


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [AMORO-3451] Fix kubernetes optimizer pod is frequently being killed [amoro]

Posted by "czy006 (via GitHub)" <gi...@apache.org>.
czy006 commented on code in PR #3458:
URL: https://github.com/apache/amoro/pull/3458#discussion_r1994634361


##########
charts/amoro/values.yaml:
##########
@@ -278,6 +278,7 @@ optimizer:
       #kube-config-path: "~/.kube/config"
       image: "apache/amoro:latest"
       pullPolicy: "IfNotPresent"
+      jvm.heap.ratio: "0.8"
   extra: []

Review Comment:
   I'm worried that our parameters will increase, can we put them into extra? Perhaps there is an additional map maintenance.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org