feat(Pool): acquire timeout, borrow validation and connection age bounds

[Yaraslaut]

Closes #562.

What the issue asked for

DataMapper/Pool.hpp had a more sophisticated waiter model than its peers but none of the operational controls that keep a pool healthy in production: no acquire timeout, no validation on borrow, no idle eviction, no max lifetime, and no retry when a broken connection is checked out. The consequences called out were a connection killed by a firewall or a failover being handed to the next caller, and an exhausted BoundedWait pool blocking the calling thread forever with no diagnostic.

What changed

All four controls live in PoolConfig, which is a non-type template parameter, so they are resolved at compile time. Every added member is defaulted — existing configurations compile unchanged.

Acquire timeout. Acquire(std::chrono::milliseconds) returns std::expected<PooledDataMapper, PoolError>. A timed-out acquirer removes itself from the waiter FIFO, so a connection returned afterwards reaches the next real waiter rather than being handed into a dead node and lost. wait_for evaluates its predicate under the mutex and reports the final value, so a false result proves the node is still Parked and no hand-off can be in flight — the de-registration is race-free rather than merely unlikely to race. The overload also exists on BoundedOverflow and UnboundedGrow, where it always succeeds, so generic code can take a timeout without knowing the strategy.

AcquireAsync deliberately has no timeout overload: it suspends a coroutine rather than occupying a thread, and there is no timer infrastructure in Async/ to expire it against. That overlaps #553.

Validation on borrow (validateOnBorrow, default Yes). Checks SqlConnection::IsAlive() before a connection leaves the pool, discarding a dead one and serving the caller from the next idle or a fresh connection — this is also the "retry on broken connection" item.

maxIdleTimeMs retires a connection that sat idle past the bound. This is not redundant with validation: IsAlive() reads the driver-local SQL_ATTR_CONNECTION_DEAD attribute, and drivers commonly only set it once an operation has already failed, so the half-open socket left behind when a firewall drops a flow without FIN or RST still passes the check. An idle bound set below the firewall's timeout removes that failure mode instead of trying to detect it.

maxLifetimeMs retires by total age, counted from creation and surviving checkout. This targets connections that are alive but no longer appropriate — after a failover or a rolling restart a pooled connection stays bound to the old node, reports itself healthy, and nothing else in the pool would ever move it.

Both bounds default to disabled. A default recycle window would silently change behaviour for every existing deployment, and the right value depends on the infrastructure the connections traverse. They are millisecond counts rather than std::chrono::duration because std::chrono::duration keeps its representation private and so is not a structural type; PoolConfig::MaxIdleTime() / MaxLifetime() read them back as durations.

Design notes

Retirement is lazy. Expired connections are dropped as they leave or enter the idle set. No housekeeping thread is added, so the destructor contract ("the pool must outlive every acquirer") is untouched and no Pool — including the process-wide GlobalDataMapperPool and every short-lived test pool — grows a thread. The trade-off, documented in docs/connection-pool.md: a pool that goes completely idle keeps its sockets until the next Acquire. Correctness holds regardless, since an expired connection is discarded rather than handed out, but the pool is not a mechanism for releasing connections during quiet periods.

One deliberate bypass. A direct hand-off to a parked BoundedWait waiter skips both the bounds and the liveness check. A waiter blocks on a predicate that only a hand-off satisfies, so retiring the connection there would strand it, and building a replacement means a DataMapper construction that can throw inside a noexcept Return. Such a connection was in active use moments earlier and is checked normally the next time it comes out of the idle set.

Structural change. Pooled connections now carry creation and idle timestamps, so the pool stores an Entry rather than a bare unique_ptr<DataMapper>; this threads through PooledDataMapper, WaiterNode and the async awaitable. PooledDataMapper's public surface (operator->, Get()) is unchanged. Retired entries are moved out and destroyed after the pool mutex is released, so ODBC disconnects never run under the lock.

Incidental fix. BoundedWait incremented _checkedOut before constructing the DataMapper, so a failing connect permanently consumed a slot. The increment now happens only once the connection stands up.

Risk assessment

• Behaviour change: validateOnBorrow defaults to Yes, so every pool now performs one SQLGetConnectAttr per borrow. It is driver-local with no round trip, and it can only cause a dead connection to be replaced rather than handed out. Set it to ValidateOnBorrow::No to opt out.
• Per-DBMS: the logic is dialect-independent — no SqlQueryFormatter involvement and no SQL emitted. SQL_ATTR_CONNECTION_DEAD is core ODBC.
• Threading: the locking protocol is unchanged; the new state is per-entry timestamps read under the same mutex. The one new wait is wait_for on the existing per-waiter CV.
• ABI/API: additive. No new virtual was added to SqlLogger (that would have broken downstream implementors), so retirement reuses the existing hooks.
• Performance: with the default config (TracksTime == false) no clock is ever read — the timestamp fields are written as default-constructed values and never compared. if constexpr compiles out every disabled bound.

Testing

src/tests/DataMapper/PoolHealthTests.cpp, 10 cases. The time-based ones use an injected clock (SetClock, under BUILD_TESTS, alongside the existing IdleCount/WaiterCount seams) rather than sleeping.

The health tests avoid weak assertions about object identity: they close a pooled connection so it is observably dead, disable validation, and then assert on liveness of what comes back — a live connection proves the bound retired the dead one and built a replacement. One test asserts the opposite direction (validation disabled really does hand the broken connection back), pinning the baseline the others rely on.

Covered: timeout on an exhausted pool; no ghost waiter left behind after a timeout; timeout satisfied by a concurrent return; timeout always succeeding on non-waiting strategies; dead connection replaced with validation on; dead connection returned with validation off; idle bound; lifetime bound retiring on return; lifetime measured as total age across repeated churn rather than reset per return; BoundedWait releasing capacity when it retires.

Results

	Full suite	dbtool suite
sqlite3	1411 passed, 1 skipped	pass
mssql2022 (Docker)	1409 passed, 3 skipped	pass
postgres (Docker 16)	1410 passed, 2 skipped	pass

Skips are pre-existing UNSUPPORTED_DATABASE cases. [Pool] (28 cases, including the pre-existing pool tests) was additionally run 40× on sqlite3 and 20× on postgres to check for flakiness: zero failures.

Compilers. Built and run under clang-debug (ASan + UBSan, -Werror), clean. gcc-release is a Linux-only preset and this machine's g++ is Apple clang, so a full GCC build was not possible; instead GCC 15 was used for a front-end check of Pool.hpp instantiating all three growth strategies against six PoolConfig combinations — clean under -Wall -Wextra -Wconversion. That covers the GCC-specific risks that matter here (structural-type acceptance of the extended PoolConfig as an NTTP, two-phase lookup in the new templates) but not codegen differences, so CI's GCC release legs are still the real check.

clang-tidy did not run — the clang-debug preset reported [clang-tidy] Not found. on this machine.

Not touched: SqlBackup/ConnectionPool.cpp has the same unbounded cv.wait. The issue cites it as evidence but scopes the fix to PoolConfig, and it is an internal detail of the backup pipeline, so it is left for a follow-up.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!