8346834: Tests failing with -XX:+UseNUMA due to "NUMA support disabled" warning

[swati-sha]

Hi All,

A number of tests launch VMs and read the output of the sub-process. The changes in JDK-8205051 mean the warning message "NUMA support disabled: Only a single NUMA node is available" is printed when running the tests -XX:+UseNUMA on system that only have one node, this breaks several tests. After update in some tests, so far, the failures are with:

java/util/logging/LoggingDeadlock2.java
tools/jar/modularJar/Basic.java

As a fix have changed logging level from "log_warning" to "log_info" when UseNUMA flag is disabled.

Thanks,
Swati Sharma
Intel

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8346834: Tests failing with -XX:+UseNUMA due to "NUMA support disabled" warning (Bug - P3)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22948/head:pull/22948
$ git checkout pull/22948

Update a local copy of the PR:
$ git checkout pull/22948
$ git pull https://git.openjdk.org/jdk.git pull/22948/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22948

View PR using the GUI difftool:
$ git pr show -t 22948

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22948.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back swati-sha! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@swati-sha The following label will be automatically applied to this pull request:

• hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (508312fb)

[kstefanj]

I think this is a bit unfortunate. I saw the comment in the bug around the wording for the flag UseNUMA:

Use NUMA if available

I don't fully agree that it's wrong to issue a warning (just because is says "if available"), it would be wrong to issue an error and terminate the process. I see the warning as a way to inform the user that the performance feature they configured the process to use couldn't be used. If this is instead communicated as a log statement on info-level almost nobody will see it.

Especially for the newly added case, where we disable NUMA when the cpu and memory nodes mismatch, I think the warning could be helpful to users.

Looking at tools/jar/modularJar/Basic.java I see that it already has code to handle "VM warnings", so adding support for "UL warnings" would likely be fine there. The other test listed above can't handle any type of unexpected logging and also fails with -Xlog:gcso adding something like: -Xlog:all=off to the child process in that test would avoid a lot of failures caused by unexpected logs.

I don't oppose this change but wanted to share my view on it.

[dwhite-intel]

Those are good points @kstefanj. This is touching a lot of areas including testing, logging (it would be handy to run tests with GC logs I'd think), as well as the "principle of least surprise" for the user.

We'll leave this topic to you and @AlanBateman, @dholmes-ora and others with more experience in the wider issues. We've implemented the message both as a warning and as "info" and are happy to have either one.

[dholmes-ora]

Just to reinforce what I've already stated, if I ask for something "if available" I don't expect to get a warning when it is not available - I knew it might not be available. If using NUMA is a potential performance boost then naturally people will want to use it, if it is available. My concern is about the potential impact on end-users who set this in their deployment settings, not the tests that shone the light on the potential problem (for which I'm grateful the tests did in fact fail!).

Warning about a NUMA misconfiguration is a different matter - that is a problem that needs someone's attention to fix.

[dholmes-ora]

So this is too coarse. Some reasons for disabling NUMA may require a warning because something is actually wrong. I only want to see the new warning for "Only a single NUMA node is available" not be a warning.