RFR(s): 8228580: DnsClient TCP socket timeout

[Pavel Rappo]

Milan,

This looks like a good start. Please consider

1. Setting TOLERANCE to 5 seconds
2. Getting the second time mark immediately after the query returns (i.e. do not waste your time in DNSTestUtils.debug(attrs))
3. Making the test parametric instead of hardcoded for the DEFAULT_DNS_CLIENT_TIMEOUT
4. Running the test for at least 2 different values of the timeout, e.g.:

    * @run main TcpTimeout
    * @run main TcpTimeout -Dcom.sun.jndi.dns.timeout.initial=5000

As for you question, I'm not sure how we would be able to communicate the fact that the response is truncated to the user. You could try to ask this on the net-dev mailing list.

-Pavel

> On 18 Sep 2019, at 14:25, Milan Mimica <milan.mimica at gmail.com> wrote:
> 
> Hi Pavel
> 
> Sure. Here is the incremental change:
> http://cr.openjdk.java.net/~mmimica/8228580/webrev.03/
> 
> What actually bothers me from the beginning is the truncated response.
> The TXT attribute, a String, prints "A very popular h", but does not
> equals("A very popular h"), because of some stray bytes. I guess it's
> because of how DNS response parsing works. I can imagine how this
> could cause problems to users. I think, at least, we should have a way
> to tell the user that the response is truncated, and the payload is
> partial/invalid.
> 
> 
> On Tue, 17 Sep 2019 at 15:09, Pavel Rappo <pavel.rappo at oracle.com> wrote:
>> 
>> Milan,
>> 
>> While the CSR is being processed, could we maybe think of some additional testing for that change? Otherwise, that test seems kind of anemic. It makes sure that the query doesn't hang, but that's about it. It doesn't check that the timeout is respected. I was wondering if you could propose some way of testing that.
>> 
>>> On 17 Sep 2019, at 09:55, Pavel Rappo <pavel.rappo at oracle.com> wrote:
>>> 
>>> I have filed the CSR:
>>> 
>>>   https://bugs.openjdk.java.net/browse/JDK-8230965
>>> 
>>>> On 13 Sep 2019, at 11:21, Pavel Rappo <pavel.rappo at oracle.com> wrote:
>>>> 
>>>> Here's the latest webrev accumulating all the changes we've discussed so far:
>>>> 
>>>>  http://cr.openjdk.java.net/~prappo/8228580/webrev.03/
>>>> 
>>>> If people are okay with that I will proceed to creating a CSR.
>> 
> 
> 
> -- 
> Milan Mimica