8313256: Exclude failing multicast tests on AIX#15074
8313256: Exclude failing multicast tests on AIX#15074TOatGithub wants to merge 8 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back TOatGithub! A progress list of the required criteria for merging this PR into |
|
@TOatGithub To determine the appropriate audience for reviewing this pull request, one or more labels corresponding to different subsystems will normally be applied automatically. However, no automatic labelling rule matches the changes in this pull request. In order to have an "RFR" email sent to the correct mailing list, you will need to add one or more applicable labels manually using the /label pull request command. Applicable Labels
|
|
/label net |
|
@TOatGithub |
Webrevs
|
|
Are the DatagramChannel tests passing on this platform? DatagramChannel supports multicasting. |
tstuefe
left a comment
There was a problem hiding this comment.
Hi Thomas,
seems okay mostly, small nits remain, see inline remarks.
BTW you can test this on AIX via jtreg -exclude:<problemlist file> -l <tests>. [1] I often do this since I keep getting the Problemlist syntax wrong.
Cheers, Thomas
test/jdk/ProblemList.txt
Outdated
| ############################################################################ | ||
| # java/foreign | ||
|
|
||
| # jdk_foreign |
There was a problem hiding this comment.
The heading on the sections are usually test group names so "jdk_foreign" is correct, but I don't know what this change is doing in this PR.
There was a problem hiding this comment.
Thanks guys for pointing this out to a newbie! Would you expect me to add a NOOP PR with correct labeling?
Regards
Thomas
There was a problem hiding this comment.
Thanks guys for pointing this out to a newbie! Would you expect me to add a NOOP PR with correct labeling?
Regards
Thomas
You can if you want to. I'm unemotional :-)
If you do, you can mark it as "trivial" (as in, mention in the PR description its a "Trivial" change, which - when accepted by reviewers - means just one review is needed and it is exempt from the usual "wait 24hrs before pushing" rule.
There was a problem hiding this comment.
So if @AlanBateman does not insist, I'd keep it as is and hopefully rember for next time ;-)
There was a problem hiding this comment.
I don't have a strong opinion either way. I think the main thing is that I suspect you'll need to exclude more tests as there are several tests for DatagramChannel that join multicast groups and some of these likely assume they can join an IPv4 multicast group when cases where the DatagramChannel is created with the no-arg open method (and IPv6 is enabled) or with the IPV6 protocol family.
There was a problem hiding this comment.
I've created https://bugs.openjdk.org/browse/JDK-8313404 to extract the fix for jdk_foreign from this PR. It's cleaner, also for backporting, although it incurs some process overhead.
There was a problem hiding this comment.
As discussed, please remove this part of the change.
We currently exclude these two tests on AIX in our private exclude list: We should probably add them here, too. |
java/nio/channels/DatagramChannel/SendReceiveMaxSize.java doesn't do multicasting but maybe it is excluded on that platform for some other reason? There are several DatagramChannel tests that do exercise multicast and I suspect they will fail on platforms that don't allow dual IPv4/IPv6 sockets to join IPv4 multicast groups. |
I checked. SendReceiveMaxSize times out, probably for some other reason. So it should not be excluded here. But AfterDisconnect fails due to the IPv4/IPv6 issue, so this fits here. |
|
I can confirm this and will add DatagramChannel soon. SendReceiveMaxSize never returns from a native call to recvfrom... |
That's fine. The tests that I expected to see listed are tests such as BasicMulticastTests, MulticastSendReceiveTests, AdaptorMulticasting and several others but maybe they are passing on this platform. |
…red in deticated PR
|
@TOatGithub this pull request can not be integrated into git checkout patch-1
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
|
@TOatGithub This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@tstuefe, @RealCLanger) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
|
fyi: iirc, sendmaxsize stuff fails when it exceeds 4G in /tmp, asm by default, that is the size of /tmp on AIX systems configured by the ansible playbooks. There can be other issues - also resolved with the correct AIX settings (iirc, no settings for maxbuf, and max tcp and udp buffer sizes. imho: correct to include the transfer size tests, but when they became 2x 2.1 G (which becomes 4.2G) - these tests failed until /tmp was increased to 5G. Unlike many Linux systems, "rootvg" on AIX is several, interdependently sized filesystems. |
|
/integrate |
|
@TOatGithub |
|
/sponsor |
|
Going to push as commit 98a915a. |
|
@RealCLanger @TOatGithub Pushed as commit 98a915a. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
|
I see this has been integrated, were you able to confirm that the DC multicast tests are passing on this platform? |
|
Yes, the tests that we didn't exclude here seem to work. There are a few network related tests still excluded on AIX in SAP's test system but these seem to have different symptoms. We're working on analyzing these and if necessary report the issues in JBS. |
exclude multicast and related tests on AIX mentioned in https://bugs.openjdk.org/browse/JDK-8308807 until this is fixed.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/15074/head:pull/15074$ git checkout pull/15074Update a local copy of the PR:
$ git checkout pull/15074$ git pull https://git.openjdk.org/jdk.git pull/15074/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 15074View PR using the GUI difftool:
$ git pr show -t 15074Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/15074.diff
Webrev
Link to Webrev Comment