On Fri, 6 Sep 2024 16:28:36 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?).

test/jdk/com/sun/jndi/dns/ConfigTests/TimeoutWithEmptyDatagrams.java line 62:

> 60:     private static final int DNS_CLIENT_MIN_TIMEOUT = 50;
> 61: 
> 62:     private long startTime;

startTime does not need to be an instance variable: consider removing it and 
make it a local variable in the runTest() method.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20892#discussion_r1747445029

Reply via email to