Skip to content

pg: a connect and its startup end at CommandTimeoutMs instead of waiting forever - #318

Merged
MDA2AV merged 4 commits into
mainfrom
fix/pg-redis-open-deadline
Oct 12, 2026
Merged

MDA2AV merged 4 commits into
mainfrom
fix/pg-redis-open-deadline

Conversation

@MDA2AV

@MDA2AV MDA2AV commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

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. ConnectAsync waited 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. WaitForConnectionAsync starts 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. CommandTimeoutMs did not help, because it only sees commands on connections already in _connections.

ConnectAsync now arms a ring timeout of CommandTimeoutMs alongside the open. If the open is still running when it fires, it calls shutdown(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: _opening drops, 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 = 0 still disables it, and the option's docs now say it also bounds opens.

Sweep can't do this with what the pool already holds. An open in progress is not in _connections, _opening is 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 passed
  • pg: a connect the server never completes fails once CommandTimeoutMs has passed
  • pg: a pool whose first open the server never answers connects on its next attempt

The silent server is a FakeServer that accepts and never answers. Each test asserts the step its error names, so the connect test can't pass on a stalled startup:

  • For the connect step, the listener has a backlog of 0 and one connection already queued, so the kernel drops the SYN.
  • The pool tests use a pool of one. A new harness helper, Forwarder, passes the fake server's later connections to the real postgres. The query must come back through one of those.
main this PR
pg startup never answered still waiting after 10 s 127.0.0.1:<port> did not answer the startup within 1000 ms
pg connect never completed (SYN dropped) still waiting after 10 s 127.0.0.1:<port> did not accept the connection within 1000 ms
pool of one, first open never answered query still waiting after 10 s 42, through the second open

Suites: 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 ConnectAsync and the options' docs. With #292, the handshake's own loops still exit before the catch disposes the connection.

FakeServer.cs is byte-identical to the one in the message-length, SCRAM and RESP-depth branches. Forwarder.cs is new here.

…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.
@MDA2AV MDA2AV mentioned this pull request Oct 12, 2026
@MDA2AV MDA2AV changed the title pg, redis: a connect and its startup or handshake end at CommandTimeoutMs instead of waiting forever pg: a connect and its startup end at CommandTimeoutMs instead of waiting forever Oct 12, 2026
…dline

# Conflicts:
#	tests/Ioxide.Tests.Pg/Program.cs
@MDA2AV
MDA2AV merged commit f311ca1 into main Oct 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant