Repository navigation
pg: a connect and its startup end at CommandTimeoutMs instead of waiting forever - #318
Merged
Merged
Conversation
…utMs instead of waiting forever Nothing bounded opening a connection. A server that accepted the TCP connection and never answered the pg startup, or a redis AUTH or SELECT, held ConnectAsync forever. A connect whose SYN was never answered waited out the kernel's retries, about two minutes. The pool coalesces opens: the slow path starts one only when none is in flight, and the ticker replenishes only then too. So one open that never finished held off every later one, and a pool left without a live connection parked its queries for good, even after the server recovered. ConnectAsync now arms a ring timeout of CommandTimeoutMs next to the open. If the open is still running when it fires, it shuts the socket down, which completes whatever ring op the open is parked on (connect, send or recv). The open then fails with an error naming the step that stalled, and the pool's existing failure path frees the slot for the next attempt. A finished or failed open disarms it first, so a late expiry never touches an fd that may have been reused. CommandTimeoutMs = 0 still disables it. Sweep cannot do this with what the pool holds: an open in progress is not in _connections, and _opening is only a count. New tests, PgOpenTests and RedisOpenTests, with a 1000 ms timeout. A FakeServer that accepts and never answers must fail the pg startup and the redis handshake (Database 1, so the open sends a SELECT). A listener with a full backlog, which drops the SYN, must fail the pg connect step. For each client, a pool of one whose first open is never answered must connect on its next attempt: the FakeServer forwards later connections to the real server through a new harness Forwarder. Confirmed both ways: all five time out (10 s) on f298af0, and pass with the fix.
…dline # Conflicts: # tests/Ioxide.Tests.Pg/Program.cs
This was referenced Oct 12, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now pg-only. The Redis half went with
ioxide.redis, which #337 drops; main is merged in, and the Pg suite passes (9).A pg pool whose server accepted a connection and then never answered stopped connecting for good: queries waiting for a connection hung forever, even after the server recovered.
Nothing bounded opening a connection.
ConnectAsyncwaited as long as it took for the TCP connect, then for the pg startup. A server that accepts and stays silent (a hung backend, a stuck proxy) holds it there forever. A SYN that is never answered waits out the kernel's retries, about two minutes. The pool coalesces opens.WaitForConnectionAsyncstarts an open only when none is in flight, and the ticker replenishes only then too, so one open that never finished held off every later one. A pool with no live connection then parked every query until the client gave up.CommandTimeoutMsdid not help, because it only sees commands on connections already in_connections.ConnectAsyncnow arms a ring timeout ofCommandTimeoutMsalongside the open. If the open is still running when it fires, it callsshutdown(fd, SHUT_RDWR). That completes whatever ring op the open is parked on: a pending connect (SYN_SENT), a send, or a recv. The open then fails with an error naming the step that stalled. From there the pool's existing failure path runs:_openingdrops, waiters wake, and the next attempt starts. A finished or failed open disarms the timeout before anything else, so a late expiry never touches an fd that may have been reused.CommandTimeoutMs = 0still disables it, and the option's docs now say it also bounds opens.Sweepcan't do this with what the pool already holds. An open in progress is not in_connections,_openingis only a count, and the only thing that ends an armed op is a shutdown of that attempt's own socket. Tracking attempts would need a new list. So instead each attempt holds its own deadline: one ring timeout SQE, plus a socket reference and a flag in a small object that lives as long as the attempt.Tests
New, in
PgOpenTests(1000 ms timeout):pg: a startup the server never answers fails once CommandTimeoutMs has passedpg: a connect the server never completes fails once CommandTimeoutMs has passedpg: a pool whose first open the server never answers connects on its next attemptThe silent server is a
FakeServerthat accepts and never answers. Each test asserts the step its error names, so the connect test can't pass on a stalled startup:Forwarder, passes the fake server's later connections to the real postgres. The query must come back through one of those.127.0.0.1:<port> did not answer the startup within 1000 ms127.0.0.1:<port> did not accept the connection within 1000 ms42, through the second openSuites: Pg 9 with main merged in (2026-10-12); Unit 63 (1 pending, as on main) when first run, and Pg passed three more times in a row then.
Not a hot path: one timeout SQE per connect attempt, and nothing per command.
No overlap with #292, which merges cleanly: this touches only
ConnectAsyncand the options' docs. With #292, the handshake's own loops still exit before the catch disposes the connection.FakeServer.csis byte-identical to the one in the message-length, SCRAM and RESP-depth branches.Forwarder.csis new here.