Skip to content

perf(worker): one pooled connection per poll cycle, one statement of session setup - #3501

Merged
nicoloboschi merged 2 commits into
mainfrom
fix/worker-claim-connection-ceremony
Aug 14, 2026
Merged

perf(worker): one pooled connection per poll cycle, one statement of session setup#3501
nicoloboschi merged 2 commits into
mainfrom
fix/worker-claim-connection-ceremony

Conversation

@nicoloboschi

@nicoloboschi nicoloboschi commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Fixes #3499.

The problem

The worker claim fabric acquired a pooled connection per active schema. Every acquire runs the pool's setup= callback (the session GUCs — asyncpg wipes them with RESET ALL on release, so they must be re-applied), and every release runs RESET ALL / UNLISTEN * / CLOSE ALL / pg_advisory_unlock_all(). Behind a transaction-mode pooler each of those statements is its own server-side transaction, so the ceremony multiplied by the number of flagged schemas: ~12 statements per schema-visit for 2 useful queries, and a commit rate an order of magnitude above the useful work.

The fix

1. One connection per poll cycle. claim_batch acquires once and runs the active-schema scan plus every per-schema claim on that connection. Acquires drop from active_schemas per cycle to 1, taking the setup/reset ceremony with them.

Each schema's claim still opens its own transaction, so FOR UPDATE SKIP LOCKED semantics and lock hold times are unchanged — the claims are sequential, and a claim's row locks are released when that schema's claim commits, not at the end of the cycle. The progress logger's scan moves inside the connection it was already acquiring, for the same reason.

2. Single-round-trip session setup. The pool's setup callback issues one SELECT set_config($1,$2,false), … instead of N separate SETs — 4 transactions per acquire become 1.

Some of those GUCs are extension-provided (hnsw.ef_search, pg_trgm.similarity_threshold) and may not exist on the cluster. A single statement fails as a whole, so on error it falls back to applying them one at a time, skipping only the ones the server rejects — the same tolerance the per-SET try/except blocks had.

Verified against a live cluster that set_config(..., false) is session-scoped and applies identically to SET for all four GUC shapes in play (core, extension, and search_path with "$user"), and that the fallback still applies the good settings when one name in the batch is rejected.

3. HINDSIGHT_API_DB_SESSION_SETUP_ON_ACQUIRE (default true, behaviour unchanged). The per-acquire re-apply exists because RESET ALL wipes the GUCs — that is exactly right for a plain asyncpg pool. But a deployment that already pins the same settings on the role or the database (ALTER ROLE … SET) gets them back from RESET ALL, so the re-apply resends values that are already correct: a pure round trip per acquire, and behind a transaction-mode pooler a pure transaction. Setting the flag to false drops the per-acquire hook and keeps the open-time one, so a connection is still configured when it is created.

Two things deliberately stay outside the trade-off:

  • application_name is always re-applied. pgbouncer never re-issues it after RESET ALL (fix(db): keep the DSN's application_name across pool reuse (blank pg_stat_activity behind pgbouncer) #3491), so losing its per-acquire hook would silently un-attribute every acquire after the first. With the flag off, setup= re-asserts the name and nothing else.
  • On the vchord text-search backend the set includes search_path (bm25_catalog, tokenizer_catalog). Unlike the tuning GUCs, losing that one fails recall outright (type "bm25vector" does not exist) rather than degrading it — called out in the docs and the env template so operators pin it before turning the flag off.

The issue's bonus observation — folding the busy-banks pre-query into the claim SQL as a CTE — is deliberately not in this PR; it's an optimization of the useful queries, not of the ceremony.

Tests

  • tests/test_worker_claim_connection_reuse.py (new) — acquire count is 1 per claim_batch() and does not grow with the number of active schemas (1 schema vs 20); each schema's claim still runs in its own transaction, all on the cycle's single connection.
  • tests/test_db_abstraction.pyapply_session_settings emits exactly one statement with the values bound (not interpolated) and false for session scope; emits nothing when there is nothing to set; falls back to per-setting application when the batched statement fails, applying the rest and skipping only the rejected name.
  • tests/test_db_session_setup_flag.py (new) — the env var parses (including rejecting an ambiguous value); setup= is the callback when on and None when off; with an application_name in the DSN and the flag off, setup= re-asserts the name and does not run the session GUCs, while init= still applies both.

…session setup

The worker's claim fabric acquired a pooled connection *per active schema*.
Every acquire runs the pool's setup callback (the session GUCs, since asyncpg
wipes them with RESET ALL on release) and every release runs
RESET ALL / UNLISTEN * / CLOSE ALL / pg_advisory_unlock_all. Behind a
transaction-mode pooler each of those statements is its own server-side
transaction, so the ceremony multiplied by the number of flagged schemas:
~12 statements per schema-visit for 2 useful queries, ~463 acquire/release
cycles/s at 12 workers x ~22 schemas, and a commit rate an order of magnitude
above the useful work.

Two changes, either of which removes most of the cost:

1. claim_batch acquires once for the whole cycle and runs the active-schema
   scan plus every per-schema claim on that connection. Each schema's claim
   still opens its own transaction, so FOR UPDATE SKIP LOCKED semantics and
   lock hold times are unchanged. The progress logger's scan moves inside the
   connection it already held, for the same reason.

2. The pool's session setup issues one SELECT set_config(...) instead of N
   separate SETs. Extension GUCs (hnsw.ef_search, pg_trgm.similarity_threshold)
   may not exist on the cluster and would fail the batched statement as a
   whole, so it falls back to applying them one at a time, skipping only the
   ones the server rejects — the same tolerance the per-SET try/except had.

Fixes #3499
The pool wires its init callback as `setup=` as well as `init=`, so the
session GUCs are re-applied on every acquire. That is required for a plain
asyncpg pool — releasing a connection runs RESET ALL, which wipes them — but
it is pure waste for deployments that pin the same settings on the role or
database, since RESET ALL restores them to exactly the values we would resend.
Behind a transaction-mode pooler that wasted round trip is also its own
server-side transaction, which is the cost #3499 measured.

HINDSIGHT_API_DB_SESSION_SETUP_ON_ACQUIRE=false drops the per-acquire hook and
keeps the open-time one, so a connection is still configured when it is
created. application_name is deliberately outside the trade-off: pgbouncer
never re-issues it after RESET ALL (#3491), so it keeps its per-acquire hook
either way.

On the vchord text-search backend the set includes search_path
(bm25_catalog, tokenizer_catalog), where losing the value fails recall
outright rather than degrading it — called out in the docs and the env
template so operators pin it before turning the flag off.

Default is true — unchanged behaviour.

Refs #3499
@nicoloboschi
nicoloboschi force-pushed the fix/worker-claim-connection-ceremony branch from bd6ba09 to d7d7def Compare August 14, 2026 14:43
@nicoloboschi
nicoloboschi merged commit 32b90cc into main Aug 14, 2026
108 checks passed
@nicoloboschi
nicoloboschi deleted the fix/worker-claim-connection-ceremony branch August 14, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants