search

eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 721 forks source link

Should -XX:+ContainerSupport be the default outside containers? #4707

Open DanHeidinga opened 5 years ago

DanHeidinga commented 5 years ago

In https://github.com/eclipse/openj9/issues/2263, it was decided to enable container support by default based on the belief that this only modified things in a container:

Given:

  • it's already enabled in Adopt's docker images,
  • the start up effect is minimal outside a container, and
  • it's a no-op outside a container,

I'm onboard with enabling this by default.

Originally posted by @DanHeidinga in https://github.com/eclipse/openj9/issues/2263#issuecomment-427224689

When reviewing #2954, I was surprised to see that omrsysinfo_cgroup_get_enabled_subsystems includes some cgroup systems are enabled even outside a container. See the example javacore for a host system: https://github.com/eclipse/openj9/files/2602877/javacore.container.m2g.txt

I'm now wondering if this code should be gated by a call to omrsysinfo_is_running_in_container(): https://github.com/eclipse/openj9/blob/87f128a0e7e924befcb2841eafbc8c951e915527/runtime/vm/jvminit.c#L1930-L1938

Does anyone have concerns with the current code? Does this need the in_container() check?

FYI @dinogun @ashu-mehra

ashu-mehra commented 5 years ago

I think this is intentional because JVM can be running in cgroups on the host as well, in which case it should honor the restrictions imposed by the cgroup on memory and CPU. So whether the JVM is running in container, or directly on host in cgroups, it would use the cgroup limits.

On another note -XX:+UseContainerSupport is actually a misnomer. A better name would have been -XX:+UseCgroupSupport.

bharathappali commented 4 years ago

On another note -XX:+UseContainerSupport is actually a misnomer. A better name would have been -XX:+UseCgroupSupport.

@ashu-mehra I feel -XX:+UseContainerSupport would cover a generic use case of running in containers, Currently its jus Linux but in future OpenJ9 might support container awareness in windows based containers (which are not Cgroup based) etc (namely OpenVZ containers, kata containers etc) So -XX:+UseContainerSupport would be helpful to keep it common for all platforms.

Please correct me if I'm wrong.