Skip to content

Commit

Permalink
8299858: [Metrics] Swap memory limit reported incorrectly when too large
Browse files Browse the repository at this point in the history 
Reviewed-by: stuefe
  • Loading branch information
@jerboaa
jerboaa committed Jan 26, 2023
1 parent 28545dc commit 64ddf95
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 3 deletions.
15 changes: 13 additions & 2 deletions src/java.base/linux/classes/jdk/internal/platform/CgroupMetrics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,11 @@ public long getMemoryFailCount() {
@Override
public long getMemoryLimit() {
long subsMem = subsystem.getMemoryLimit();
long systemTotal = getTotalMemorySize0();
assert(systemTotal > 0);
// Catch the cgroup memory limit exceeding host physical memory.
// Treat this as unlimited.
if (subsMem >= getTotalMemorySize0()) {
if (subsMem >= systemTotal) {
return CgroupSubsystem.LONG_RETVAL_UNLIMITED;
}
return subsMem;
Expand All @@ -142,7 +144,15 @@ public long getTcpMemoryUsage() {

@Override
public long getMemoryAndSwapLimit() {
return subsystem.getMemoryAndSwapLimit();
long totalSystemMemSwap = getTotalMemorySize0() + getTotalSwapSize0();
assert(totalSystemMemSwap > 0);
// Catch the cgroup memory and swap limit exceeding host physical swap
// and memory. Treat this case as unlimited.
long subsSwapMem = subsystem.getMemoryAndSwapLimit();
if (subsSwapMem >= totalSystemMemSwap) {
return CgroupSubsystem.LONG_RETVAL_UNLIMITED;
}
return subsSwapMem;
}

@Override
Expand Down Expand Up @@ -185,5 +195,6 @@ public static Metrics getInstance() {

private static native boolean isUseContainerSupport();
private static native long getTotalMemorySize0();
private static native long getTotalSwapSize0();

}
13 changes: 13 additions & 0 deletions src/java.base/linux/native/libjava/CgroupMetrics.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* questions.
*/
#include <unistd.h>
#include <sys/sysinfo.h>

#include "jni.h"
#include "jvm.h"
Expand All @@ -43,3 +44,15 @@ Java_jdk_internal_platform_CgroupMetrics_getTotalMemorySize0
jlong page_size = sysconf(_SC_PAGESIZE);
return pages * page_size;
}

JNIEXPORT jlong JNICALL
Java_jdk_internal_platform_CgroupMetrics_getTotalSwapSize0
(JNIEnv *env, jclass ignored)
{
struct sysinfo si;
int retval = sysinfo(&si);
if (retval < 0) {
return 0; // syinfo failed, treat as no swap
}
return (jlong)si.totalswap;
}
42 changes: 41 additions & 1 deletion test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
* @run driver jdk.test.lib.helpers.ClassFileInstaller -jar whitebox.jar jdk.test.whitebox.WhiteBox
* @run main/othervm -Xbootclasspath/a:whitebox.jar -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI TestMemoryAwareness
*/
import java.util.function.Consumer;
import jdk.test.lib.containers.docker.Common;
import jdk.test.lib.containers.docker.DockerRunOptions;
import jdk.test.lib.containers.docker.DockerTestUtils;
Expand Down Expand Up @@ -96,7 +97,9 @@ public static void main(String[] args) throws Exception {
true /* additional cgroup fs mounts */
);
testOSMXBeanIgnoresMemLimitExceedingPhysicalMemory();
testOSMXBeanIgnoresSwapLimitExceedingPhysical();
testMetricsExceedingPhysicalMemory();
testMetricsSwapExceedingPhysical();
testContainerMemExceedsPhysical();
} finally {
if (!DockerTestUtils.RETAIN_IMAGE_AFTER_TEST) {
Expand Down Expand Up @@ -183,6 +186,13 @@ private static void testOperatingSystemMXBeanAwareness(String memoryAllocation,

private static void testOperatingSystemMXBeanAwareness(String memoryAllocation, String expectedMemory,
String swapAllocation, String expectedSwap, boolean addCgroupMounts) throws Exception {
Consumer<OutputAnalyzer> noOp = o -> {};
testOperatingSystemMXBeanAwareness(memoryAllocation, expectedMemory, swapAllocation, expectedSwap, false, noOp);
}

private static void testOperatingSystemMXBeanAwareness(String memoryAllocation, String expectedMemory,
String swapAllocation, String expectedSwap, boolean addCgroupMounts,
Consumer<OutputAnalyzer> additionalMatch) throws Exception {

Common.logNewTestCase("Check OperatingSystemMXBean");

Expand All @@ -191,6 +201,7 @@ private static void testOperatingSystemMXBeanAwareness(String memoryAllocation,
"--memory", memoryAllocation,
"--memory-swap", swapAllocation
)
.addJavaOpts("-esa")
// CheckOperatingSystemMXBean uses Metrics (jdk.internal.platform) for
// diagnostics
.addJavaOpts("--add-exports")
Expand Down Expand Up @@ -228,9 +239,9 @@ private static void testOperatingSystemMXBeanAwareness(String memoryAllocation,
} catch(RuntimeException ex) {
out.shouldMatch("OperatingSystemMXBean\\.getFreeSwapSpaceSize: 0");
}
additionalMatch.accept(out);
}


// JDK-8292541: Ensure OperatingSystemMXBean ignores container memory limits above the host's physical memory.
private static void testOSMXBeanIgnoresMemLimitExceedingPhysicalMemory()
throws Exception {
Expand All @@ -239,6 +250,35 @@ private static void testOSMXBeanIgnoresMemLimitExceedingPhysicalMemory()
testOperatingSystemMXBeanAwareness(badMem, hostMaxMem, badMem, hostMaxMem);
}

private static void testOSMXBeanIgnoresSwapLimitExceedingPhysical()
throws Exception {
long totalSwap = wb.hostPhysicalSwap() + wb.hostPhysicalMemory();
String expectedSwap = Long.valueOf(totalSwap).toString();
String hostMaxMem = getHostMaxMemory();
String badMem = hostMaxMem + "0";
final String badSwap = expectedSwap + "0";
testOperatingSystemMXBeanAwareness(badMem, hostMaxMem, badSwap, expectedSwap, false, o -> {
o.shouldNotContain("Metrics.getMemoryAndSwapLimit() == " + badSwap);
});
}

private static void testMetricsSwapExceedingPhysical()
throws Exception {
Common.logNewTestCase("Metrics ignore container swap memory limit exceeding physical");
long totalSwap = wb.hostPhysicalSwap() + wb.hostPhysicalMemory();
String expectedSwap = Long.valueOf(totalSwap).toString();
final String badSwap = expectedSwap + "0";
String badMem = getHostMaxMemory() + "0";
DockerRunOptions opts = Common.newOpts(imageName)
.addJavaOpts("-XshowSettings:system")
.addDockerOpts("--memory", badMem)
.addDockerOpts("--memory-swap", badSwap);

OutputAnalyzer out = DockerTestUtils.dockerRunJava(opts);
out.shouldContain("Memory Limit: Unlimited");
out.shouldContain("Memory & Swap Limit: Unlimited");
}

// JDK-8292541: Ensure Metrics ignores container memory limits above the host's physical memory.
private static void testMetricsExceedingPhysicalMemory()
throws Exception {
Expand Down

5 comments on commit 64ddf95

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@MBaesken
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 64ddf95 Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MBaesken the backport was successfully created on the branch backport-MBaesken-64ddf953 in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 64ddf953 from the openjdk/jdk repository.

The commit being backported was authored by Severin Gehwolf on 26 Jan 2023 and was reviewed by Thomas Stuefe.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev.git backport-MBaesken-64ddf953:backport-MBaesken-64ddf953
$ git checkout backport-MBaesken-64ddf953
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev.git backport-MBaesken-64ddf953

@MBaesken
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 64ddf95 Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MBaesken the backport was successfully created on the branch backport-MBaesken-64ddf953 in my personal fork of openjdk/jdk11u-dev. To create a pull request with this backport targeting openjdk/jdk11u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 64ddf953 from the openjdk/jdk repository.

The commit being backported was authored by Severin Gehwolf on 26 Jan 2023 and was reviewed by Thomas Stuefe.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk11u-dev:

$ git fetch https://github.com/openjdk-bots/jdk11u-dev.git backport-MBaesken-64ddf953:backport-MBaesken-64ddf953
$ git checkout backport-MBaesken-64ddf953
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk11u-dev.git backport-MBaesken-64ddf953

⚠️ @MBaesken You are not yet a collaborator in my fork openjdk-bots/jdk11u-dev. An invite will be sent out and you need to accept it before you can proceed.

Please sign in to comment.