Repository navigation
UN-4254 [PERF] Run PG-queue ops in autocommit and slow idle polling - #2328
Merged
Merged
Conversation
The PG-queue client and result backend ran every single-statement operation inside psycopg2's implicit BEGIN ... COMMIT, so each claim (including empty polls), lease renewal, ack, enqueue and result read/write cost three round trips. Their owned connections now run in autocommit and send only the statement. create_pg_connection() itself is unchanged: pg_barrier's reconnect-retry relies on it staying non-autocommit, and the reaper, scheduler and leader election run multi-statement transactions on their own connections. Injected connections keep the caller's mode. Idle consumers polled every 0.1s after a task and backed off only to 2s, so idle DB load scaled with slots x queues. Defaults are now POLL_INTERVAL=0.5 and BACKOFF_MAX=5.0 (still env-tunable); BACKOFF_MAX stays well under HEALTH_STALE_SECONDS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGgUm6WuXrgzYq1WhrFcX7
main's #2349 already made PgQueueClient's owned connection autocommit, so client.py takes main's version. The consumer backoff comment is corrected for read_any (one claim per cycle per slot, not per queue), and the injected-connection test that duplicated main's is dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ckend Remediation of review findings F1, F2, F4, F5, F6, F7, F8: - F1: PgResultBackend._cursor treats a bare DatabaseError as a dead connection, as PgQueueClient._cursor does, since rollback() sends nothing under autocommit; docstring corrected. - F2: pin server-side backend termination on the result backend (live DB). - F4, F5, F8: backoff comment and README rows no longer claim extra pollers shorten pickup, and say a handled message (not a claim) resets the backoff. - F6: drop the PgQueueClient autocommit unit test that duplicated main's. - F7: pin the default backoff cap below the default health-stale threshold. Unstract-Remediation: remediation-deep iteration 1
…ients Behaviour-preserving cleanup (simplify pass): - connection.is_dead_after_autocommit_error holds the bare-DatabaseError rule and its rationale once; PgQueueClient._cursor and PgResultBackend._cursor both call it. is_connection_dead is unchanged. - The live-DB reconnect tests share an owned-backend fixture and a read-back helper; the redundant TEST_DB_HOST setdefault is dropped (pg_conn already sets it). Unstract-Remediation: remediation-deep iteration 2
…de does The confirming review showed a server-terminated connection raises OperationalError and is marked closed, so it was never only the new helper that caught it. Reword is_dead_after_autocommit_error's docstring, the PgQueueClient._cursor comment and both result-backend test comments to the narrow case the helper actually adds (a bare DatabaseError on a connection psycopg2 still reports open), and have the termination test assert the OperationalError it really sees. Unstract-Remediation: remediation-deep iteration 2
johnyrahul
force-pushed
the
ccr-679ddaa3-wvlx3r
branch
from
October 22, 2026 10:46
a3a6a90 to
bea677d
Compare
johnyrahul
marked this pull request as ready for review
October 22, 2026 10:48
|
Contributor
 [Medium impact] The PR appears safe to merge; no actionable failures were found.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Unstract test resultsPer-group results
Critical paths
|
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.
What
PgResultBackendturns on autocommit on the connections it opens itself, so each result store / get / forget is sent as one statement, with noBEGIN/COMMITround trips.is_dead_after_autocommit_error()helper inconnection.pydecides when an owned autocommit connection must be dropped after an error. BothPgQueueClient._cursorandPgResultBackend._cursoruse it.POLL_INTERVAL=0.1/BACKOFF_MAX=2.0to0.5/5.0.PgQueueClient's own autocommit already landed onmainin #2349 (UN-4176). This PR mergesmainand does not repeat that change.Why
From the UN-4136 load tests (see UN-4254):
BEGIN/COMMITwas about 34–38% of all statements under load. Most of it wrapped operations that are already one atomic statement. UN-4176 [FIX] Cut idle PG-queue polling to one autocommit claim per cycle #2349 removed this for the queue client; the result backend still paid it on every result write and read.BACKOFF_MAXand follows worker count, not work. Idle dev namespaces were running about 150 claims/s each on the shared staging instance.How
PgResultBackend.connsetsautocommit = Trueonly when it opens its own connection. Every statement it runs is a single idempotentSELECT,INSERT … ON CONFLICT DO NOTHINGorUPDATE, so nothing relied on two statements committing together.rollback()sends nothing to the server, so it no longer exposes a dead connection that psycopg2 still reports as open.is_dead_after_autocommit_error()therefore also treats a barepsycopg2.DatabaseErroras a dead connection. This is the rulePgQueueClient._cursoralready had inline; it now lives in one place.is_connection_dead()is unchanged, so no retry site starts retrying a bareDatabaseError.OperationalErrorand psycopg2 marks it closed._DEFAULT_POLL_INTERVALand_DEFAULT_BACKOFF_MAXare raised. The consumer comment and the README table describe the backoff as it works: it starts after a cycle that handles nothing, doubles up toBACKOFF_MAX, and resets when a cycle handles a message.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
create_pg_connection()is unchanged:pg_barrier's reconnect-retry depends on it not being in autocommit. The reaper, scheduler and leader election run multi-statement transactions on their own connections and are untouched. Connections passed in by a caller keep the caller's mode and are never dropped.DatabaseError. If that error was only an unmapped statement error, the cost is one needless reconnect. It is not retried, the same as before this PR.WORKER_PG_QUEUE_CONSUMER_BACKOFF_MAX. 5 s stays well underHEALTH_STALE_SECONDS(60 by default, 180+ in every deployed config).Database Migrations
Env Config
WORKER_PG_QUEUE_CONSUMER_POLL_INTERVAL(0.1 → 0.5) andWORKER_PG_QUEUE_CONSUMER_BACKOFF_MAX(2.0 → 5.0). Deployments that set them explicitly are unaffected.BACKOFF_MAXmust still be at leastPOLL_INTERVAL, so a deployment that sets onlyBACKOFF_MAXbelow 0.5 would now fail at startup; no compose file or chart sets either variable.Relevant Docs
workers/queue_backend/pg_queue/README.mdconsumer config table is updated.Related Issues or PRs
result_backend.py).Dependencies Versions
Notes on Testing
test_pg_queue_client.py,test_pg_result_backend.py,test_pg_queue_consumer.pyagainst a local Postgres 15 with thepg_queuemigrations applied: 217 passed. 8 more fail identically onmainlocally (a stale local schema and venv) and were excluded.DatabaseErrordrops the owned connection, while a SQLSTATE-mapped statement error keeps it;pg_terminate_backendheals on the owned autocommit connection (the read drops the handle, the write self-heals);PgQueueClientsend / read / set_vt / delete round trip;BACKOFF_MAXstays below the defaultHEALTH_STALE_SECONDS.Screenshots
N/A
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code