Fix a recursive update with a ConcurrentHashMap in `RefreshingAddre…

[ikhoon]

…ssResolver`

Motivation:

I saw an infinite test that never terminates on our CI build with Java 8
That is caused by a recursive update which invalidates a cache when failed to query DNS recodes.
See #3528 for details

Modifications:

• Invalidate a cache asynchronously to avoid recursive updates

Result:

• You no longer see an infinity loop or an IllegalStateException: Recurse update
  when invalidating a DNS cache.
• Fixes Infinity loop on cache miss in RefreshingAddressResolver #3528

[codecov]

Codecov Report

Merging #3530 (2206733) into master (031bc8e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #3530      +/-   ##
============================================
- Coverage     73.93%   73.91%   -0.02%     
+ Complexity    14206    14203       -3     
============================================
  Files          1245     1245              
  Lines         54187    54187              
  Branches       6929     6929              
============================================
- Hits          40061    40051      -10     
- Misses        10586    10596      +10     
  Partials       3540     3540              

Impacted Files	Coverage Δ	Complexity Δ
...corp/armeria/client/RefreshingAddressResolver.java	76.42% <100.00%> (+1.42%)	20.00 <1.00> (+1.00)
...om/linecorp/armeria/client/HttpSessionHandler.java	73.14% <0.00%> (-8.58%)	49.00% <0.00%> (-4.00%)
...ria/common/stream/PublisherBasedStreamMessage.java	81.87% <0.00%> (-1.88%)	20.00% <0.00%> (-1.00%)
...p/armeria/internal/common/eureka/InstanceInfo.java	51.75% <0.00%> (-0.88%)	23.00% <0.00%> (-1.00%)
.../linecorp/armeria/client/Http1ResponseDecoder.java	66.66% <0.00%> (-0.61%)	44.00% <0.00%> (-1.00%)
...a/com/linecorp/armeria/client/HttpChannelPool.java	78.41% <0.00%> (-0.28%)	67.00% <0.00%> (ø%)
...corp/armeria/common/logging/DefaultRequestLog.java	76.63% <0.00%> (+0.25%)	222.00% <0.00%> (+1.00%)
...armeria/internal/common/CancellationScheduler.java	78.09% <0.00%> (+0.41%)	86.00% <0.00%> (+1.00%)
...rmeria/common/stream/ConcatArrayStreamMessage.java	80.21% <0.00%> (+2.19%)	10.00% <0.00%> (ø%)
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 031bc8e...2206733. Read the comment docs.

[ben-manes]

Alternatively, you might be able to return null from the loader to not cache the entry? My naive reading of the code leads me to believe that would have a similar effect, as the future's value is resolved to null instead.

Caffeine also offers an AsyncCache and custom expiration. I'd have hoped that some of this code could be simpler by using that, e.g. by not scheduling the invalidation yourself. In that instance the entry could be resolved as expired immediately to cause it to be evicted.

[ikhoon]

Alternatively, you might be able to return null from the loader to not cache the entry? My naive reading of the code leads me to believe that would have a similar effect, as the future's value is resolved to null instead.

The returned CompletableFuture is usually resolved lazily except for some error cases. In that case, it should be difficult to run null.

Caffeine also offers an AsyncCache and custom expiration. I'd have hoped that some of this code could be simpler by using that, e.g. by not scheduling the invalidation yourself. In that instance the entry could be resolved as expired immediately to cause it to be evicted.

Nice suggestion. AsyncCache and custom expiration look promising. Let us take a look how it works and is applicable.

[trustin]

Thanks, @ikhoon!

[trustin]

Nice suggestion. AsyncCache and custom expiration look promising. Let us take a look how it works and is applicable.

This sounds better to me.

[minwoox]

Thanks, @ikhoon!
AsyncCache also sounds good.

[ikhoon]

AsyncCache also sounds good.

Filed an issue for this. #3547

[trustin]

Thanks, @ikhoon. Let's merge this first and then migrate to AsyncCache. Special thanks to @ben-manes for chiming in. 🙇