aws: use caching DNS resolver for S3 HTTP client to reduce lookup churn#6584
Conversation
fulmicoton-dd
force-pushed
the
paul-masurel/aws-caching-dns-resolver
branch
2 times, most recently
from
fba0474 to
de7d9e1
Compare
The AWS SDK's default HTTP client resolves DNS via hyper's uncached GaiResolver, causing redundant blocking getaddrinfo lookups under high connection churn (e.g. many concurrent S3 range requests). Swap in a Hickory-based resolver with a configurable minimum TTL floor to absorb bursts while still following endpoint IP rotations.
fulmicoton-dd
force-pushed
the
paul-masurel/aws-caching-dns-resolver
branch
from
de7d9e1 to
6a8c9f0
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a8c9f0602
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
If we fix the connection churn issue, do we actually need DNS caching? |
Not really, but yes. |
Co-authored-by: Adrien Guillo <adrien.guillo@gmail.com>
Hickory only speaks the DNS protocol (plus /etc/hosts), so endpoints resolved through other NSS sources (mDNS, LDAP, custom nsswitch.conf plugins) would otherwise fail to resolve now that it's used for all AWS service configs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0761fe8acd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| heck,https://github.com/withoutboats/heck,MIT OR Apache-2.0,Without Boats <woboats@gmail.com> | ||
| hermit-abi,https://github.com/hermit-os/hermit-rs,MIT OR Apache-2.0,Stefan Lankes | ||
| hex,https://github.com/KokaKiwi/rust-hex,MIT OR Apache-2.0,KokaKiwi <kokakiwi@kokakiwi.net> | ||
| hickory-proto,https://github.com/hickory-dns/hickory-dns,MIT OR Apache-2.0,The contributors to Hickory DNS |
There was a problem hiding this comment.
The new Hickory resolver pulls in additional crates recorded in quickwit/Cargo.lock (hickory-net, prefix-trie, critical-section, ndk-context, system-configuration, and system-configuration-sys), but this updated CSV only adds hickory-proto/hickory-resolver around here. Release artifacts using LICENSE-3rdparty.csv will under-report bundled third-party code, so please regenerate the license file or add all new transitive dependencies.
Useful? React with 👍 / 👎.
| let mut builder = | ||
| TokioResolver::builder_tokio().context("failed to read system DNS configuration")?; | ||
| let options = builder.options_mut(); | ||
| options.positive_min_ttl = Some(DNS_CACHE_MIN_TTL); |
There was a problem hiding this comment.
When users configure an S3-compatible endpoint via set_endpoint_url (the S3 client still uses this shared HTTP client), this unconditionally raises every positive DNS TTL to 60 seconds. Endpoints that publish low TTLs for failover or load-balancer rotation will keep stale IPs in the process cache for up to a minute, whereas the previous getaddrinfo path re-resolved on new connections; please skip the floor or expose an opt-out/config for custom endpoints.
Useful? React with 👍 / 👎.
fulmicoton-dd
force-pushed
the
paul-masurel/aws-caching-dns-resolver
branch
5 times, most recently
from
c919abc to
fc5e910
Compare
The main benefit of hickory is: read TTL from DNS entry, but we override that anyway + being really async. The cons was that it does not hit libc based resolve (reading hosts etc.). This commit also adds support for HTTP_PROXY.
fulmicoton-dd
force-pushed
the
paul-masurel/aws-caching-dns-resolver
branch
from
fc5e910 to
9e57b47
Compare
4 participants
Summary
GaiResolver, causing redundant blockinggetaddrinfolookups under high connection churn (e.g. many concurrent S3 range requests).HickoryDnsResolverinquickwit-awsbacked by a caching Hickory resolver, with a configurable minimum TTL floor (QW_S3_DNS_CACHE_MIN_TTL_SECS, default 60s) so short-TTL S3 records are still cached long enough to absorb bursts.Test plan
bash scripts/check_license_headers.shcargo clippy --workspace --all-features --testscargo +nightly fmt --all -- --checklocalhostvia the new resolver