microsoft / microsoft/mssql-rs

mssql-tds: no stale-on-failure DNS fallback (msodbcsql parity gap — a transient resolution failure fails the connect outright)

Open
#457 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.