Race condition bugfix in the dnscache.#6586
Conversation
I ended up not using hickory for the cache in order to support all of the libc subtleties (resolv.conf) Unfortunately, upon expiration, and until the dns entry is updated, all new connection create triggers a DNS update. Because our DNS is slightly slow, this was causing a significantly inflated (and confusing) number of DNS calls. The hickory approach on this was to block all callers. I am not fond of this solution either: it would synchronize GET for no reason at all, and our TTL is pretty arbitrary. There is no need to pay for DNS latency at all. This PR chooses to run DNS refresh asynchronously when the TTL is expired and the entry is stale.
![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: f7482d6769
ℹ️ 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".
| tokio::task::spawn(async move { | ||
| let Ok(socket_addrs) = tokio::net::lookup_host((host.as_str(), 0)).await else { | ||
| quickwit_common::rate_limited_error!(limit_per_min = 10, %host, "failed to refresh DNS"); | ||
| return; |
There was a problem hiding this comment.
When an uncached lookup reaches this error path, should_resolve() has already pushed next_resolve_attempt five seconds into the future while the cached value is still empty. Any application retry during that cooldown therefore does not run lookup_host again and only waits for an update that no task is producing, so a transient first DNS failure can turn several quick retries into guaranteed timeout failures.
Useful? React with 👍 / 👎.
fulmicoton-dd
force-pushed
the
paul.masurel/dnscache-bugfix
branch
from
3cae4f6 to
1378be6
Compare
3 participants
I ended up not using hickory for the cache
in order to support all of the libc subtleties (resolv.conf)
Unfortunately, upon expiration, and until the dns entry is updated, all new connection create triggers a DNS update.
Because our DNS is slightly slow, this was causing a significantly inflated (and confusing) number of DNS calls.
The hickory approach on this was to block all callers.
I am not fond of this solution either: it would synchronize GET for no reason at all, and our TTL is pretty arbitrary. There is no need to pay for DNS latency at all.
This PR chooses to run DNS refresh asynchronously when the TTL is expired and the entry is stale.