microsoft / microsoft/mssql-rs
mssql-tds: no stale-on-failure DNS fallback (msodbcsql parity gap — a transient resolution failure fails the connect outright)
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 53
- Forks
- 14
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 137
Description
Describe the bug
Follow-up from review discussion on PR #432 (mssql-tds: resolve TCP connect hostnames asynchronously — the AB#47704 hang fix), specifically from https://github.com/microsoft/mssql-rs/pull/432#discussion_r3900555148 and the reply that researched msodbcsql's actual source (Sql/Common/DK/sni/src/dns_cache.cpp:65).
msodbcsql's GetCachedAddrInfo calls the real getaddrinfo on every resolution — it is not a hot-path/dedup cache. The cache is consulted only when a fresh resolution fails, returning the last-known-good addresses instead of failing the connect outright:
DWORD ret = GetAddressInfo(node, service, hints, &origAddrInfo);
if (ret)
{
// DNS failure, try the cache
if (CACHEADDRINFOW *p = DNSCache::get(node)) { *res = p; ret = 0; }
}
else
{
// DNS success, add to cache
DNSCache::add(node, *res = new DNSCacheEntry(origAddrInfo));
}
mssql-tds has no equivalent: if resolution fails (e.g. a transient DNS outage), the connect attempt fails outright, even if the target was successfully resolved and connected to moments earlier and its address is very unlikely to have changed.
Expected behavior (parity with msodbcsql)
On a resolution failure, fall back to the most recently successful resolution for the same (host, port), if one is cached and reasonably fresh, and attempt the connect with that instead of failing immediately. On a resolution success, refresh the cache entry.
Actual behavior
create_base_stream_sequential (network_transport.rs) and parallel_connect (parallel_connect.rs) call tokio::net::lookup_host directly with no fallback; a resolution error propagates straight to the caller.
Suggested direction
A small per-(host, port) cache (scope TBD — see #451 for the same open question there) storing the last successful resolution, consulted only as a fallback when a fresh lookup_host call fails. Needs design discussion: cache eviction/TTL for the fallback entries themselves (msodbcsql's appears to be unbounded/session-lifetime — worth confirming), interaction with redirection/MultiSubnetFailover/AlwaysOn listener failover (stale-but-served addresses could be actively wrong during a real failover, as opposed to a transient DNS blip on an unchanged topology — these need to be distinguished), and how to surface to the caller that a connection succeeded against a cached (unverified-this-attempt) address.
Version
Commit af805bbb (PR #432 head), main branch otherwise unmodified.
Affected crate
mssql-tds
Additional context
Split out from #451 at the suggestion of the PR #432 review discussion, since it's a distinct mechanism from that issue's TTL de-dup cache proposal: this one is about resilience to resolution failures (matching an msodbcsql behavior we currently lack), not about reducing redundant successful resolutions during pool warm-up (which msodbcsql does not do either — it re-resolves on every call, so #451's dedup-cache proposal would go beyond msodbcsql parity, not just match it).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading create_base_stream_sequential in network_transport.rs and parallel_connect in parallel_connect.rs, then review the linked PR #432 discussion and issue #451 for the unresolved cache-scope questions. Define the fallback cache behavior, freshness and failover handling before implementing it. Done means successful resolutions are remembered and transient lookup failures can use an appropriate cached result without breaking redirection or MultiSubnetFailover.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases, networking
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100