RFR: 8339538: Wrong timeout computations in DnsClient [v11]

Aleksei Efimov aefimov at openjdk.org
Wed Oct 9 17:26:14 UTC 2024


On Mon, 7 Oct 2024 09:00:28 GMT, Aleksei Efimov <aefimov at 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/3251a5e8...c58e34cc

The changes in this PR are expected to help with intermittent failures of the `com/sun/jndi/dns/ConfigTests/Timeout.java` test. 
Therefore, I'm marking  [JDK-8220213](https://bugs.openjdk.org/browse/JDK-8220213) as solved by this PR too.

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

PR Comment: https://git.openjdk.org/jdk/pull/20892#issuecomment-2402889782


More information about the core-libs-dev mailing list