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/860af487...c58e34cc

good job ... thanks for considering my feedback

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

Marked as reviewed by msheppar (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20892#pullrequestreview-2360760459

Reply via email to