Add non-blocking DNS for the async API via c-ares event-loop integration#324
Draft
bjosv wants to merge 5 commits into
Draft
Add non-blocking DNS for the async API via c-ares event-loop integration#324bjosv wants to merge 5 commits into
bjosv wants to merge 5 commits into
Conversation
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
When built with USE_CARES=1 (make) or -DENABLE_CARES=ON (CMake), DNS resolution in the sync API uses c-ares with a poll loop bounded by connect_timeout (defaulting to 5s). This prevents indefinite hangs when DNS is slow or unresponsive. The c-ares path uses ARES_OPT_SOCK_STATE_CB for fd tracking, and a short-lived channel per resolve call. IPv4/IPv6 fallback behavior is preserved. Without enabling c-ares, behavior is unchanged (plain getaddrinfo). Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
When c-ares is enabled, the async connect path defers DNS until the event adapter is attached. The libevent adapter initiates async DNS on attach, with c-ares fds and timers driven by the event loop. On DNS completion, socket() + connect() proceed as normal and the fd is registered with the event loop for connect completion. Adapters without c-ares hooks fall back to blocking DNS. Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
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
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.
Even with
c-aresenabled for timeout-bounded DNS, the async API still blocks the calling thread during DNS resolution. This happens because DNS is resolved insidevalkeyAsyncConnectWithOptions()before returning, the same blocking call used by the sync API. For applications using async I/O to avoid blocking (e.g. serving many connections on a single thread), a slow DNS lookup stalls the entire event loop.This PR builds on the c-ares sync DNS support (#323) and makes DNS fully non-blocking for the async API. When the libevent adapter is used, DNS resolution is driven by the event loop, so no thread blocks during DNS.
See commit "Add c-ares async DNS resolve for non-blocking connect", the other commits are from #323.
How it works:
When
c-aresis enabled and an async connection is made, DNS is no longer resolved before the function returns. Instead:connect_timeout. These adapters can be upgraded to full async DNS by implementing the c-ares hooks.connect()is called (non-blocking, as usual). The event loop then handles connect completion.Design decisions:
127.0.0.1). Adone/failedflag pattern ensures cleanup happens safely outside the c-ares callback, preventing use-after-free.VALKEY_DNS_BLOCKING_FALLBACKmacro makes it trivial for adapters to opt in to the blocking fallback, one line per adapter._EL_ADD_READ/_EL_ADD_WRITEmacros suppress event registration while DNS is pending, preventing operations on an invalid fd.Custom adapters:
Third-party adapters must add
VALKEY_DNS_BLOCKING_FALLBACK(ac)to their attach function, or implement the c-ares hooks for full async DNS. See the documentation indocs/standalone.md.Testing:
ct_async_dnsintegration test exercising both the libevent async path and the standalone connect flow