-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
8269657: Test java/nio/channels/DatagramChannel/Loopback.java failed: Unexpected message #19314
Conversation
…rmittent Signed-off-by: sendaoYan <yansendao.ysd@alibaba-inc.com>
👋 Welcome back syan! A progress list of the required criteria for merging this PR into |
@sendaoYan 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 435 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. 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 (@dfuch) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
@sendaoYan The following label will be automatically applied to this pull request:
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. |
Webrevs
|
No objection to add the keyword. From a quick look at the logs then it hints that it may be a kernel issue rather than a JDK issue, e.g. here one of the failures in the logs you attached:
I assume veth87b01b2 is a virtual interface. When IP_MULTICAST_LOOP is enabled the 5-byte multicast datagram is looped as expected. When IP_MULTICAST_LOOP is disabled then it appears to be looped again. So this may require digging into the kernel to better understand this. It would be interesting to know if it's always IPv6 where the failures happen. |
I reporduced this intermittent failure on a ECS, so veth87b01b2 is a virtual interface. I will close IPv6 and reproduce this failure. After I close IPv6, and this failure can not repoeduce, then maybe the failure alway happend in IPv6. |
The GHA test runner report two failures:
|
@sendaoYan This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
Hi, can anyone review this PR. |
…we receive whose payload doesn't match what we expect
|
Please give me some time to test the proposed changes. |
Okey. |
Hi, can you maybe refrain from adding the intermittent keyword to the test? In our test setup at SAP this would mean that the test is not run any longer. |
@dfuch I run this test after the this patch 20k times and all passed, maybe the |
/issue JDK-8332535 |
/issue JDK-8332535 |
@dfuch Only the author (@sendaoYan) is allowed to issue the |
Actually - maybe we should use JDK-8269657 for this change, since it fixes the test failure. You can grab it from @Michael-Mc-Mahon, I don't think he will mind :-) FWIW - I have launched some repeated tests in our CI - so far this looks promising :-) |
Okey. |
@sendaoYan |
/issue remove JDK-8332535 |
@sendaoYan The issue |
/issue JDK-8269657 |
@sendaoYan This issue is referenced in the PR title - it will now be updated. |
Thanks for the patient guidance and review. |
@sendaoYan |
Hi @sendaoYan - thanks for integrating all the feedback. I can sponsor this change - unless you already have a sponsor? |
No, I need your sponsor, thanks. |
/sponsor |
Going to push as commit c798316.
Your commit was automatically rebased without conflicts. |
@dfuch @sendaoYan Pushed as commit c798316. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Thanks for the sponsor. |
Hi all,
The test
test/jdk/java/nio/channels/DatagramChannel/Loopback.java
intermittent fail, the failure probability is about 2/1000.This PR mark add some entropy(System.nanoTime) to the message content, and if the payloads doesn't match then ignore it, because payloads probably receiving some stray packet sent by another process. Only if the port unmatch and the content match we consider it a failure.
The change has been verified locally, I have runed the test after the this patch 20k times and all passed. Only change the testcase, no risk.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19314/head:pull/19314
$ git checkout pull/19314
Update a local copy of the PR:
$ git checkout pull/19314
$ git pull https://git.openjdk.org/jdk.git pull/19314/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19314
View PR using the GUI difftool:
$ git pr show -t 19314
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19314.diff
Webrev
Link to Webrev Comment