microsoft / microsoft/mssql-rs

mssql-tds: connection-pool warm-up issues redundant, uncached DNS resolutions for the same host

Open
#451 2 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](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

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 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.