On Mon, 7 Oct 2024 09:00:28 GMT, Aleksei Efimov <aefi...@openjdk.org> wrote:
>> This PR proposes the following changes to address wrong timeout computations >> in the `com.sun.jndi.dns.DnsClient`: >> - The `DnsClient` has been updated to use a monotonic high-resolution (nano) >> clock. The existing `Timeout` test has also been updated to use the nano >> clock to measure observed timeout value. >> >> - The left timeout computation has been fixed to decrease the timeout value >> during each retry attempt. A new test, `TimeoutWithEmptyDatagrams`, has been >> added to test it. >> >> - The `DnsClient.blockingReceive` has been updated: >> - to detect if any data is received >> - to avoid contention with `Selector.close()` that could be called by a >> cleaner thread >> >> - The expected timeout calculation in the `Timeout` test has been updated to >> take into account the minimum retry timeout (50ms). Additionally, the max >> allowed difference between the observed timeout and the expected one has >> been increased from 50% to 67%. Taking into account 50 ms retry timeout >> decrease the maximum allowed difference is effectively set to 61%. This >> change is expected to improve the stability of the `Timeout` test which has >> been seen to fail >> [intermittentlly](https://bugs.openjdk.org/browse/JDK-8220213). If no >> objections, I'm planning to close >> [JDK-8220213](https://bugs.openjdk.org/browse/JDK-8220213) as duplicate of >> this one. >> >> JNDI/DNS jtreg tests has been executed multiple times (500+) to check if the >> new and the modified tests are stable. No failures been observed (so far?). > > Aleksei Efimov has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 16 additional > commits since the last revision: > > - Merge remote-tracking branch 'origin' into > JDK-8339538_DnsClient_nanoTime_And_InfiniteLoop > - improve reporting for unrecoverable exceptions > - Add comment suggested by Daniel > - Track unfulfilled timeouts during UDP queries. > Update exceptions handling. > Update TCP client to use nano time source. > - set min timeout to 0; set max allowed timeout to 2x expected timeout in > tests > - set max allowed value for retries to 30 > - update tests to calculate allowed timeout range (max is updated to 75%), > print it and use it for elapsed time check > - remove redundant clamp from timeoutLeft calculation > - clamp timeout and retries property values > - Measure time the caller spent waiting. Simplify timeoutLeft computation > - ... and 6 more: https://git.openjdk.org/jdk/compare/772b32cc...c58e34cc test/jdk/com/sun/jndi/dns/ConfigTests/TimeoutWithEmptyDatagrams.java line 114: > 112: }); > 113: > 114: // Run a virtual thread that will send an empty packets via > server socket similarly here the lambda functionality encapsulated in a class abstraction EmptyDatagramSender implements Runnable Thread.startVirtualThread(new EmptyDatagramSender(. . .)) I make this and the above comment as an observation ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20892#discussion_r1795595815
