resolver: cache all resolved addresses, not just the first#142
Open
zytakeshi wants to merge 1 commit into
Open
resolver: cache all resolved addresses, not just the first#142zytakeshi wants to merge 1 commit into
zytakeshi wants to merge 1 commit into
Conversation
CachingNativeResolver stored only a single SocketAddr per cache entry. A cache miss returned the full Vec<SocketAddr> from lookup_host, but a cache hit returned only the first address. SocketConnectorImpl::connect iterates the full list with a per-address connect fallback, so within the cache TTL every connection after the first to a multi-address host could only ever try one address -- if that address was unreachable (e.g. a dual-stack host whose first record is down) the connection failed even though a working address was cached. Store and return the full list on both paths so the hit path is symmetric with the miss path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CachingNativeResolver (the default resolver when no DNS config is set) caches resolution results in CachedResolveResult, which stored only a single SocketAddr per entry.
The two cache paths were asymmetric:
So the first lookup of a host returned every resolved record, while every subsequent lookup within the TTL (default 1h) returned just one address.
This matters because the consumer SocketConnectorImpl::connect iterates the full address list with a real per-address connect fallback (both the TCP and QUIC transports try each address in turn until one succeeds). With a cache hit returning a single address, that fallback has nothing left to try. The practical failure: a dual-stack host whose first record is unreachable connects on the first attempt (miss -> full list -> fallback works) but fails on every subsequent attempt for the rest of the TTL (hit -> single dead address).
Fix: store the full Vec in the cache and clone it on a hit, so the hit path is symmetric with the miss path. TTL/expiry semantics are unchanged.
Single-address callers are unaffected: resolve_single_address and the separate ResolverCache still take addrs[0], and the head of the list is identical to the previously-cached single address. A focused regression test asserts that a cache hit returns the same full list as the initial miss.