microsoft / microsoft/mssql-rs
mssql-tds: connection-pool warm-up issues redundant, uncached DNS resolutions for the same host
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](https://sqlclientdrivers.visualstudio.com/b95cf060-8083-439d-8ef1-405d5bf219d8/_workitems/edit/47704) hang fix). Not a bug in the merged fix; a separable efficiency improvement identified during that review.
`mssql-tds`'s connect paths (`create_base_stream_sequential` in `network_transport.rs`, `parallel_connect` in `parallel_connect.rs`) each independently call `tokio::net::lookup_host((host, port))` — a fresh DNS resolution (`getaddrinfo`) per connection attempt, with no caching or sharing across concurrent or sequential connects to the same `(host, port)`.
This is pre-existing (predates PR #432 — the prior blocking `std::net::ToSocketAddrs` call had the same "one resolution per connect" behavior; PR #432 changed *how* resolution runs, not *how often*).
### Where this matters
Connection-pool warm-up: when a pool opens N connections concurrently, all targeting the same SQL Server, that's N independent, identical `getaddrinfo` calls happening around the same moment:
- N redundant syscalls doing the exact same lookup.
- Since PR #432, each now also transiently borrows a thread from Tokio's shared `spawn_blocking` pool (default cap 512) for the overlapping resolution window — so peak thread usage during warm-up scales with N as well, though comfortably within the default cap for realistic pool sizes (discussed at length in PR #432's review thread — not itself considered a bottleneck for typical pool sizes, but the redundant work stacks with it).
### Suggested direction
A short-TTL cache (or in-flight de-duplication — a single resolution shared by all concurrent callers requesting the same `(host, port)`, then cached briefly) at the `mssql-tds` connect layer would cut both the redundant `getaddrinfo` calls and the peak `spawn_blocking` thread usage during pool warm-up, without changing per-connection behavior once warm.
Needs its own design discussion: cache scope (per-`ClientContext`? per-ENV, matching `mssql-odbc`'s runtime scope? global?), TTL policy, interaction with DNS changes/failover (redirection, `MultiSubnetFailover`, AlwaysOn listener IP changes — a stale cache entry could actively hurt failover scenarios), and whether de-duplicating in-flight lookups (vs. just caching completed ones) is worth the added complexity.
### Version
Commit af805bbb (PR #432 head), main branch otherwise unmodified.
### Affected crate
mssql-tds
### Additional context
Related PR discussion: https://github.com/microsoft/mssql-rs/pull/432#discussion_r3900562419 and the surrounding thread. Related but distinct from #445 (spawn_blocking DNS being uncancellable at ODBC env teardown) — that one is about a stuck/wedged resolver blocking cleanup; this one is about redundant work for a healthy, fast resolver under concurrent load.
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 with create_base_stream_sequential in network_transport.rs and parallel_connect in parallel_connect.rs, then read the related PR #432 review thread. The work needs a decision on cache scope, TTL, in-flight sharing, and failover behavior before implementation. Done means redundant lookups during concurrent pool warm-up are addressed without harming connection behavior or DNS-based failover.
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
- Needs clarification
- Newbie friendliness
- 35/100