8371629: Add diagnostic output IPSupport.printPlatformSupport(System.out) to tests in java/nio/channels directory -- part I
Add diagnostic output IPSupport.printPlatformSupport(System.out) to tests in java/nio/channels directory -- part I
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8371629: Add diagnostic output IPSupport.printPlatformSupport(System.out) to tests in java/nio/channels directory -- part I (Sub-task - P3)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28420/head:pull/28420
$ git checkout pull/28420
Update a local copy of the PR:
$ git checkout pull/28420
$ git pull https://git.openjdk.org/jdk.git pull/28420/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28420
View PR using the GUI difftool:
$ git pr show -t 28420
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28420.diff
Using Webrev
:wave: Welcome back serhiysachkov! 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.
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
@serhiysachkov The following labels will be automatically applied to this pull request:
-
build -
nio
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.
Hello Serhiy, my opinion is that we shouldn't be adding this change to every single test file.
When a test is run, typically in CI or locally, it runs as part of several other tests on a specific host. The IPSupport.printPlatformSupport(...) prints host specific details and not test specific details, so calling this in every single test would generate redundant output and would require update and future additions to every single test within the networking area.
If this information isn't already captured in the jtreg failure handlers of the JDK, then I think we should enhance those failure handlers so that this same information gets collected just once when there is a test failure or a timeout. That I think should avoid all these test updates.
Hello Serhiy, my opinion is that we shouldn't be adding this change to every single test file. When a test is run, typically in CI or locally, it runs as part of several other tests on a specific host. The
IPSupport.printPlatformSupport(...)prints host specific details and not test specific details, so calling this in every single test would generate redundant output and would require update and future additions to every single test within the networking area.If this information isn't already captured in the jtreg failure handlers of the JDK, then I think we should enhance those failure handlers so that this same information gets collected just once when there is a test failure or a timeout. That I think should avoid all these test updates.
Hi Jaikiran, this output would help during ATR failure analysis, as experience of previous ones shows there are a lot of communication going on during this period on env setup especially in IPv6 testing, so the idea is to save some time for those back and forward communications in clearing out testing environments when 3rd party is involved in testing. Plus it's a background for future changes, that will involve runs with specific JVM option as -Djava.net.PreferIPv6Addresses in IPV6 only and mixed env.
@jaikiran This is not just about test failures in IPv6 environments, but also that the test that pass are also correctly passing. It will also assist in diagnosing failures in the CI pipelines. It is to assist is determining if anomalous behaviour is due to a dynamic configuration change in a test environment. It is to ensure that there is a consistent configuration and setup during the execution of the networking tests in an IPv6 only env. Ask yourself why there are so many TCP4 connections rather than TCP4,6 connections in the failures associated with https://bugs.openjdk.org/browse/JDK-8273158 ?
I have my doubts on the usefulness of this as IPSupport.printPlatformSupport doesn't print the network configuration. If there is dynamic reconfiguration going on (and I agree that is hard to diagnose) then dumping the network configuration in the jtreg failure handler would be useful if it's not there already.
I have my doubts on the usefulness of this as IPSupport.printPlatformSupport doesn't print the network configuration. If there is dynamic reconfiguration going on (and I agree that is hard to diagnose) then dumping the network configuration in the jtreg failure handler would be useful if it's not there already.
Yes, the network configuration is printed in the jtreg handler. BUT, wrt this addition, we are not looking for network configuration (at this juncture). We wish to determine what is the "perception" of the JDK runtime, used in the test, wrt IPv4 and IPv6 support and correlate that with the configuration i.e. IPv6 only. Has it created a socket in the correct domain etc., has the OS been (accidentally) altered to enable IPv4, while it may not have it configured. If preferIPv4Stack is set true, why has a test not failed.
Additionally, this is not just for failures. It is also to allow review of successful test execution, and to ensure the consistency of behaviour within IPv6 only environments.
Prior to running test in IPv6 only environments, we run a set of sanity checks, which are set standalone tests verifying the configured environments. We capture the network configuration for comparison with the jtreg failure capture.
We do a lot of preparation to ensure the env are setup correctly. But anomalies have been observed, which have raised question on the veracity of the test results.
This change is to assist in evaluating the test JDK runtime thinks it is using and what has been configured.
It's a small addition which goes a long way to helping the analysis