Connection pooling, and a read path decision (relay#29) #103

2026-09-04T15:43:35Z

paulhitt commented

2026-09-04 15:43:35 +00:00

(Migrated from gitlab.com)

The premise needed correcting first, and it changes what the fix is for.
The issue says "every held long-poll draws from max_connections". A held poll
does NOT pin a database connection — Routes.poll borrows one per cursor query
for a few milliseconds and gives it back, which was a deliberate choice
RoutesWake.swift already explains (LISTEN would have pinned a backend per
waiter, capping simultaneous waiters at ~86, and was rejected for exactly
that). So the pressure is proportional to the QUERY rate, not to held polls —
measured at nine round trips per message in docs/COST.md. Still a good
argument for a pooler, arguably a better one, but a different one, and sizing
the pooler off the wrong premise would produce the wrong numbers.

Verified rather than assumed: the relay is compatible with transaction
pooling. No named prepared statements — postgres-nio's ordinary query path
builds an ExtendedQueryContext with no statement name, and the named
prepareStatement API is separate and unused. No LISTEN/NOTIFY. No advisory
locks; leader election is a Redis SET. No session GUCs, no temp tables. The
dozen db.transaction blocks are fine, because a transaction is the unit a
transaction pooler pools by.

PoolerCompatibilityTests keeps that true, and it is the durable half of
this change. PgBouncer migrations do not break on the day you do them — they
break months later when somebody adds an advisory lock, intermittently and
under load. The scan fails the build with the reason each construct breaks.
Proven by planting a pg_advisory_lock in a source file and watching it go red.

docker-compose.pooled.yml is an overlay. DEFAULT_POOL_SIZE 20 is backends and
is what must stay under max_connections; MAX_CLIENT_CONN 500 is clients, and
the gap between them is the multiplexing. FN_PG_MAX_CONNECTIONS_PER_LOOP goes
UP behind a pooler, 4 to 16 — the relay's connections stop being Postgres
backends and become cheap client connections. Reserve pool off, because a
reserve raises the real ceiling above the one number the file exists to bound.

The read replica is declined, and the issue names the reason itself. The
cursor query is not a report — it is how a phone learns a message exists, and
the relay cannot tell "the replica is behind" from "there is no mail" because
those are the same answer. A send commits on the primary, a poll reads a lagging
replica and returns empty, and what a person sees is a message that was
sometimes slow under load: indistinguishable from a bad network and
attributable to nothing. If a read path ever needs offloading, the candidates
are the ones where staleness is visible and harmless — the key log, the public
pages, the status page. Not the inbox.

What is owed, and docs/POOLING.md says so: the pooler has not been run
under load. Compatibility is settled and the settings are reasoned from
measured round-trip counts, but nothing here has had traffic through it. The
three steps before it goes on production are written down.

631 relay tests in 56 suites green, core paths 7/7.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

The premise needed correcting first, and it changes what the fix is for. The issue says "every held long-poll draws from max_connections". A held poll does NOT pin a database connection — Routes.poll borrows one per cursor query for a few milliseconds and gives it back, which was a deliberate choice RoutesWake.swift already explains (LISTEN would have pinned a backend per waiter, capping simultaneous waiters at ~86, and was rejected for exactly that). So the pressure is proportional to the QUERY rate, not to held polls — measured at nine round trips per message in docs/COST.md. Still a good argument for a pooler, arguably a better one, but a different one, and sizing the pooler off the wrong premise would produce the wrong numbers. Verified rather than assumed: the relay is compatible with transaction pooling. No named prepared statements — postgres-nio's ordinary query path builds an ExtendedQueryContext with no statement name, and the named prepareStatement API is separate and unused. No LISTEN/NOTIFY. No advisory locks; leader election is a Redis SET. No session GUCs, no temp tables. The dozen db.transaction blocks are fine, because a transaction is the unit a transaction pooler pools by. PoolerCompatibilityTests keeps that true, and it is the durable half of this change. PgBouncer migrations do not break on the day you do them — they break months later when somebody adds an advisory lock, intermittently and under load. The scan fails the build with the reason each construct breaks. Proven by planting a pg_advisory_lock in a source file and watching it go red. docker-compose.pooled.yml is an overlay. DEFAULT_POOL_SIZE 20 is backends and is what must stay under max_connections; MAX_CLIENT_CONN 500 is clients, and the gap between them is the multiplexing. FN_PG_MAX_CONNECTIONS_PER_LOOP goes UP behind a pooler, 4 to 16 — the relay's connections stop being Postgres backends and become cheap client connections. Reserve pool off, because a reserve raises the real ceiling above the one number the file exists to bound. The read replica is declined, and the issue names the reason itself. The cursor query is not a report — it is how a phone learns a message exists, and the relay cannot tell "the replica is behind" from "there is no mail" because those are the same answer. A send commits on the primary, a poll reads a lagging replica and returns empty, and what a person sees is a message that was sometimes slow under load: indistinguishable from a bad network and attributable to nothing. If a read path ever needs offloading, the candidates are the ones where staleness is visible and harmless — the key log, the public pages, the status page. Not the inbox. What is owed, and docs/POOLING.md says so: the pooler has not been run under load. Compatibility is settled and the settings are reasoned from measured round-trip counts, but nothing here has had traffic through it. The three steps before it goes on production are written down. 631 relay tests in 56 suites green, core paths 7/7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

paulhitt commented

2026-09-04 15:43:35 +00:00

(Migrated from gitlab.com)

assigned to @paulhitt

paulhitt commented

2026-09-04 20:40:59 +00:00

(Migrated from gitlab.com)

deleted the fix/device-row-read-twice branch. This merge request now targets the main branch

deleted the `fix/device-row-read-twice` branch. This merge request now targets the `main` branch

paulhitt commented

2026-09-04 20:41:01 +00:00

(Migrated from gitlab.com)

mentioned in commit 49a4c1a518

mentioned in commit 49a4c1a518afc8d0d7e908b4b9c71a73372a0712

paulhitt (Migrated from gitlab.com) merged commit 49a4c1a518 into main

2026-09-04 20:41:01 +00:00

paulhitt commented

2026-09-09 18:59:07 +00:00

(Migrated from gitlab.com)

changed the description

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: extant2000/flamenet-relay#103