RFR: 8304885: Reuse stale data to improve DNS resolver resiliency [v11]

[Daniel Fuchs]

On Mon, 22 May 2023 18:54:06 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> I would like to get preliminary feedback about the provided patch.
>> 
>> Discussion on net-dev@ https://mail.openjdk.org/pipermail/net-dev/2023-March/020682.html
>> 
>> One of the main issue I try to solve is how the cache handle the intermittent DNS server outages due to overloading or network connection.
>> 
>> At the moment this cache can be configured by the application using the following two properties:
>>    (1) "networkaddress.cache.ttl"(30 sec) - cache policy for successful lookups
>>    (2) "networkaddress.cache.negative.ttl"(10 sec) - cache policy for negative lookups
>> 
>> The default timeout for positive responses is good enough to "have recent dns-records" and to "minimize the number of requests to the DNS server".
>> 
>> But the cache for the negative responses is problematic. This is a problem I would like to solve. Caching the negative response means that for **10** seconds the application will not be able to connect to the server.
>> 
>> Possible solutions:
>>   1. Decreasing timeout "for the negative responses": unfortunately more requests to the server at the moment of "DNS-outage" cause even more issues, since this is not the right moment to load the network/server more.
>>   2. Increasing timeout "for the positive responses": this will decrease the chance to get an error, but the cache will start to use stale data longer.
>>   3. This proposal: it would be good to ignore the negative response and continue to use the result of the last "successful lookup" until some additional timeout.
>> 
>> The idea is to split the notion of the TTL and the timeout used for the cache. When TTL for the record will expire we should request the new data from the server. If this request goes fine we will update the record, if it fails we will continue to use the cached date until the next sync.
>> 
>> For example, if the new property "networkaddress.cache.extended.ttl" is set to 10 minutes, then we will cache a positive response for 10 minutes but will try to sync it every 30 seconds. If the new property is not set then as before we will cache positive for 30 seconds and then cache the negative response for 10 seconds.
>> 
>> 
>> RFC 8767 Serving Stale Data to Improve DNS Resiliency:
>> https://www.rfc-editor.org/rfc/rfc8767
>> 
>> Comments about current and other possible implementations:
>>  * The code intentionally moved to the separate ValidAddresses class, just to make clear that the default configuration, when the new property is not set is not changed much.
>>  * The refr...
>
> Sergey Bylokhov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   if the property is not set means do not use stale names

src/java.base/share/classes/java/net/InetAddress.java line 225:

> 223:  * <p>
> 224:  * A value of 0 (zero) or if the property is not set means do not use stale
> 225:  * names, the negative value is ignored.

Suggestion:

 * names. Negative values are ignored.

src/java.base/share/classes/java/net/doc-files/net-properties.html line 276:

> 274: 	preferable to use a stale name rather than result of unsuccessful name lookup.
> 275: 	A value of 0 (zero) or if the property is not set means do not use stale
> 276: 	names, the negative value is ignored.

I'd suggest to rephrase:

Suggestion:

	A value of 0 (zero) or if the property is not set means do not use stale
	names. Negative values are ignored.

src/java.base/share/classes/java/net/doc-files/net-properties.html line 277:

> 275: 	A value of 0 (zero) or if the property is not set means do not use stale
> 276: 	names, the negative value is ignored.
> 277: 	The default value is implementation-specific.</P>

Maybe we should say that the default value is 0 since this is what is documented in `java.security`

src/java.base/share/conf/security/java.security line 366:

> 364: # zero: do not use stale names
> 365: #
> 366: # default value is never (NEVER).

Suggestion:

# any positive value: the number of seconds to use the stale names
# zero: do not use stale names
# negative values are ignored
#
# default value is 0 (NEVER).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13285#discussion_r1209308636
PR Review Comment: https://git.openjdk.org/jdk/pull/13285#discussion_r1209303760
PR Review Comment: https://git.openjdk.org/jdk/pull/13285#discussion_r1209307516
PR Review Comment: https://git.openjdk.org/jdk/pull/13285#discussion_r1209306126