Repository navigation
pg: the timeout sweep removes the connection that timed out, not whatever its index holds afterwards - #313
Merged
Conversation
…ot whatever its index holds afterwards Sweep failed a timed-out connection's waiters through CheckTimeout and then removed and disposed _connections[i]. The waiters resume inline, so a handler that queries again picks from the pool inside the sweep, and Pick evicts the timed-out connection, which is broken, shifting the list. Sweep then disposed whatever had moved into index i: a live connection holding the queries just placed on it, which failed with "send failed: errno 9". When the timed-out connection was the last entry, the index was past the end and the ticker faulted, skipping the rest of that sweep. Sweep now holds the connection it checked, removes it by reference (a no-op if a Pick already did), and clamps the index to the list's current length. No new state. New tests, PgSweepTests and RedisSweepTests: a pool of 2 with a 1000 ms command timeout, a pg_sleep(30) or BLPOP for 30 s on index 0, and a handler that issues two more commands when the timeout fails it. The handler records that the timed-out connection is still listed when it runs, which shows it ran inline in the sweep. Confirmed both ways: on f298af0 both commands fail with errno 9 (pg and redis), and pass with the fix. With the sleep moved to index 1, f298af0 logs "ticker faulted: Index was out of range" and the fix does not.
MDA2AV
changed the title
# Conflicts: # src/clients/ioxide.pg/PgPool.cs # tests/Ioxide.Tests.Pg/Program.cs
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 (7).When a timed-out pg query was handled by issuing another one, the pool's timeout sweep disposed the live connection that the new command had just gone to, and that command failed with
send failed: errno 9.Sweepwalks_connectionsby index. For a connection whose oldest command is overdue,CheckTimeoutmarks it broken and fails its waiters. The waiters resume inline, inside the sweep. A handler that catches the timeout and queries again callsPick.Pickevicts the broken connection from_connections, which shifts the list, and returns the live one.Sweepthen removed and disposed_connections[i], which by then was the live connection carrying those new commands. When the timed-out connection was the last entry,_connections[i]was past the end instead: the ticker faulted withIndex was out of range, and the rest of that sweep, including replenishment, was skipped.Sweepnow keeps the connection it checked, removes it by reference (a no-op if aPickalready removed it), and clamps the index to the list's current length, since onePickcan evict several broken connections. No new state.Tests
New:
pg: queries made by a timed-out query's handler run on the live connection the sweep keeps(PgSweepTests)It uses a pool of 2 with a 1000 ms command timeout. The handler waits for both connections, then sends
pg_sleep(30), which lands on index 0. When the timeout fails it, the handler issues two more commands. The handler also records that the timed-out connection is still listed when its catch runs. That only holds inline in the sweep, so the test can't pass on a continuation that ran after the sweep had finished. A last request checks that the pool still answers.send failed: errno 91,2ticker faulted: Index was out of range, sweep cut shortsend failed: errno 321,2Suites: Pg 7 with main merged in (2026-10-12); Unit 63 (1 pending, as on main) when first run.
Not a hot path: the change is in the 250 ms ticker, and the extra list search runs only for a connection that timed out.
#292 does not fix this: on #292 alone,
Sweepshuts the live connection down instead of closing it, and its commands fail with EPIPE. The two conflict inSweep, because #292 rewrites the comment on thec.Dispose()line that sits between the lines changed here. To resolve, keep this structure with #292's comment. Pg passes with both merged.