Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update JVM_IsUseContainerSupport #18185

Merged

Conversation

babsingh
Copy link
Contributor

Currently, in comparison to the RI, OpenJ9 shows a different output for
-XshowSettings:system outside a container because our
JVM_IsUseContainerSupport implementation differs from the RI.

This PR matches the latest RI behaviour for JVM_IsUseContainerSupport.

The RI returns TRUE from JVM_IsUseContainerSupport IFF
-XX:+UseContainerSupport is specified. This option is enabled by
default.

Currently, we return TRUE from JVM_IsUseContainerSupport if
-XX:+UseContainerSupport is specified && we are inside a container.

The return value of JVM_IsUseContainerSupport determines the output
of -XshowSettings:system.

If JVM_IsUseContainerSupport returns FALSE, -XshowSettings:system has
the below output:

Operating System Metrics:
    No metrics available for this platform

If JVM_IsUseContainerSupport returns TRUE, -XshowSettings:system has
the below output:

Operating System Metrics:
    Provider: cgroupv2
    Effective CPU Count: 8
    CPU Period: -1
    CPU Quota: -1
    CPU Shares: -1
    List of Processors: N/A
    List of Effective Processors: N/A
    List of Memory Nodes: N/A
    List of Available Memory Nodes: N/A
    Memory Limit: Unlimited
    Memory Soft Limit: 0.00K
    Memory & Swap Limit: Unlimited
    Maximum Processes Limit: Unlimited

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

@babsingh
Copy link
Contributor Author

@pshipton @tajila I would also like to backport these changes to the Oct release. I feel that the risk is low. Please let me know if you agree on backporting these changes.

fyi @mstoodle

@mstoodle
Copy link
Contributor

mstoodle commented Sep 22, 2023

i suppose the main risk is that these changes could enable downstream code to execute outside of containers whereas previously it would have only executed in a container context. Presumably that environment is also where most of our testing runs, but I think we should run tests cross-platform before we consider to merge it especially given where we are in the release cycle. But all that's irrelevant for the backport question unless @pshipton and @tajila agree it's worthwhile to consider at this point.

@tajila
Copy link
Contributor

tajila commented Sep 22, 2023

It looks like the usage of JVM_IsUseContainerSupport is limited to jdk.internal.platform.CgroupMetrics which as far as I can tell is only used for printing System metrics in the launcher code for -XshowSettings:system

@babsingh
Copy link
Contributor Author

babsingh commented Sep 22, 2023

The cgroups Java API associated with -XshowSettings:system and JVM_IsUseContainerSupport: https://github.com/ibmruntimes/openj9-openjdk-jdk8/tree/openj9/jdk/src/linux/classes/jdk/internal/platform. Internally, OpenJ9 doesn't use this Java API; it relies upon the cgroups API from the OMR port library. From a JVM perspective, there should be no change to OpenJ9 behaviour with this PR. The impact will be seen on the class library side (extensions repo), which includes common/shared code between OpenJ9 and the RI. With this PR, we will just inherit the risk that the RI has already accepted with their implementation of JVM_IsUseContainerSupport. My guess ... any problems that show up in OpenJ9 SDKs will also exist in the RI SDKs.

@pshipton
Copy link
Member

jenkins test sanity.openjdk,extended.openjdk amac jdk11

@pshipton
Copy link
Member

jenkins test sanity.openjdk,extended.openjdk xlinux jdk11

Currently, in comparison to the RI, OpenJ9 shows a different output for
-XshowSettings:system outside a container because our
JVM_IsUseContainerSupport implementation differs from the RI.

This PR matches the latest RI behaviour for JVM_IsUseContainerSupport.

The RI returns TRUE from JVM_IsUseContainerSupport IFF
-XX:+UseContainerSupport is specified. This option is enabled by
default.

Currently, we return TRUE from JVM_IsUseContainerSupport if
-XX:+UseContainerSupport is specified && we are inside a container.

The return value of JVM_IsUseContainerSupport determines the output
of -XshowSettings:system.

If JVM_IsUseContainerSupport returns FALSE, -XshowSettings:system has
the below output:

    Operating System Metrics:
        No metrics available for this platform

If JVM_IsUseContainerSupport returns TRUE, -XshowSettings:system has
the below output:

    Operating System Metrics:
        Provider: cgroupv2
        Effective CPU Count: 8
        CPU Period: -1
        CPU Quota: -1
        CPU Shares: -1
        List of Processors: N/A
        List of Effective Processors: N/A
        List of Memory Nodes: N/A
        List of Available Memory Nodes: N/A
        Memory Limit: Unlimited
        Memory Soft Limit: 0.00K
        Memory & Swap Limit: Unlimited
        Maximum Processes Limit: Unlimited

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh
Copy link
Contributor Author

babsingh commented Sep 25, 2023

@pshipton pshipton merged commit 6484680 into eclipse-openj9:master Sep 25, 2023
2 checks passed
@pshipton
Copy link
Member

Pls go ahead with the backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants