fix(dbconn): keep the execution bounds where a pooler would drop them - #81
Draft
aparajon wants to merge 1 commit into
Draft
fix(dbconn): keep the execution bounds where a pooler would drop them#81aparajon wants to merge 1 commit into
aparajon wants to merge 1 commit into
Conversation
The engine's lock_timeout and statement_timeout are session settings sent as startup parameters. A pooler configured to ignore those parameters connects successfully and simply does not have them, so the engine runs unbounded while believing it is bounded — an ALTER that queues sits at the head of the lock queue indefinitely. Apply the bounds as SET statements on every new connection, which no pooler strips, then verify the session actually holds them. Because an idle transaction-mode pooler can hand the same backend back and report the bounds present, also prove the connection keeps one session before returning the pool: take an advisory lock, hold a transaction open on a second connection so the first is forced to rebind, and refuse when the lock does not follow. This is what a hosted PostgreSQL default connection string looks like — Supabase's is the transaction pooler — so the refusal names the session-mode endpoint as the remedy rather than failing opaquely.
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.
lock_timeoutandstatement_timeoutare how pg-sprite bounds every statement it issues (LK-2). They are session settings, and they were sent only as startup parameters — which a pooler is free to drop. PgBouncer drops anything inignore_startup_parameters, in session mode as well as transaction mode, so the connection succeeds and the bounds are simply absent. Measured against a real PgBouncer: asked forlock_timeout=3s, statement_timeout=30s, got0and0. Unbounded, reported as fine.That matters beyond a misconfiguration: hosted PostgreSQL hands out a pooled connection string by default, so this is what an ordinary adopter gets.
Three steps, each catching what the one before it cannot:
Step 3 is not redundant. In transaction mode PgBouncer runs no
server_reset_queryby default, so theSETfrom step 1 sticks on a reused backend and step 2 reports healthy on a connection whose next statement lands on a backend that never saw it. Only forcing the rebind settles it.The proof has no false positives — each reading is a fact about the connection in hand, not a guess about what sits behind it. It is one-sided the other way: an idle pooler with spare backends can still pass it, so it is a guard against the connection string a platform hands you, not a substitute for pointing pg-sprite at a session-mode endpoint. The refusal says which endpoint to use rather than failing opaquely.
LK-2: extends enforcement. The invariant text is unchanged — the bound was always required; this makes its absence detectable instead of silent.