Skip to content

Add configurable backend cleanup and Greengage integration tests - #281

Open
vadv wants to merge 33 commits into
ozontech:masterfrom
vadv:feat/configurable-backend-cleanup
Open

vadv wants to merge 33 commits into
ozontech:masterfrom
vadv:feat/configurable-backend-cleanup

Conversation

@vadv

@vadv vadv commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Add two settings: cleanup_server_connections modes off, adaptive, always, and cleanup_server_query. Both work in [general] and per pool.

  • adaptive (default). Built-in cleanup SQL runs only after a tracked session state change.
  • always. cleanup_server_query runs on every checkin of a backend that served a client. The query is required.
  • off. No session cleanup. ROLLBACK for an open transaction still runs.

Legacy true and false mean adaptive and off.

cleanup_server_query replaces the built-in RESET ROLE / RESET ALL / DEALLOCATE ALL / CLOSE ALL sequence. A good example is PgBouncer's default server_reset_query: DISCARD ALL. Validation checks the pair as a whole: always requires a query. adaptive rejects a query. A pool without its own settings inherits both from [general].

Adaptive detection

The built-in cleanup tracks these statements and answers with cleanup SQL:

Statement Cleanup
SET RESET ALL
DECLARE cursor CLOSE ALL
SQL PREPARE DEALLOCATE ALL
LISTEN UNLISTEN *
CREATE TEMP TABLE DISCARD TEMP

Every cleanup batch also releases advisory locks with pg_advisory_unlock_all().

Motivation:

  • SQL PREPARE and an extended-protocol Parse share one statement namespace. Without tracking, the next client on the same backend failed with 42P05 duplicate_prepared_statement.
  • A pooled backend's socket is not read. Queued NotificationResponse messages reached the client that checked the backend out next.
  • A temporary table survived checkins in both pooling modes.

Not tracked: SELECT ... INTO TEMP and CREATE TEMP TABLE AS SELECT. Current PostgreSQL completes them with the inner query's tag. Temporary objects inside functions carry no tag. Use always with suitable SQL for these.

Behavior

  • A failed cleanup retires the backend. Responses already sent to the client stay delivered, including a successful COMMIT.
  • A NOTICE from the cleanup query goes to the client. It is not an error and does not retire the backend.
  • A client RESET or DISCARD ALL suppresses built-in cleanup in adaptive. It never suppresses always.
  • A session-level advisory lock survives only until a cleanup batch runs. In transaction pooling, treat advisory locks as per-transaction.

Tests

  • BDD scenarios cover the leaks before the fix: temp tables, LISTEN subscriptions, cross-client notifications, SQL PREPARE collisions, advisory locks.
  • Other scenarios cover config inheritance and reload, prepared statements, startup parameters, asynchronous protocol boundaries, and cleanup metrics.
  • make test-greengage runs @greengage scenarios on a pinned Greengage 7.5 image with one coordinator and two primary segments. These tests also run in CI.

Scope

  • Runtime changes: 18 files, +693 / −105 lines. Tag classifier and cleanup batch, config parsing, backend retire path, the pg_doorman_server_cleanup_total metric.
  • Tests: 24 files, +2770 / −56 lines. BDD features and steps, plus in-file unit tests.
  • Documentation: 10 files, +175 / −102 lines. Pool reference, concept pages, config examples generator, changelog.

Other changes

  • New metric pg_doorman_server_cleanup_total with labels user, database, result (ok / error).
  • Updated pg_doorman.toml, pg_doorman.yaml, and the English and Russian documentation.
  • Launchpad publishing now runs only for releases.
  • Removed a missing frontend-file dependency that caused repeated Rust rebuilds.

Upgrade notes

  1. The default behavior does not change.
  2. Old configs with true and false keep working.
  3. SHOW and config dumps omit pool-level cleanup_server_connections when the pool does not set its own. Treat the field as optional in parsers.
  4. Greengage users: set cleanup_server_connections = "always" and a cleanup query with DISCARD TEMP. Greengage DISCARD ALL leaves temporary tables on segments. See the pool reference for an example.
  5. Advisory locks are released by every cleanup batch in adaptive mode. If a client holds session-level advisory locks across transactions, use session pooling or off.
  6. In transaction pooling a temporary table lives for one transaction: the cleanup follows the CREATE at the next checkin. Keep a transaction open across the statements that use the table, or use session pooling.

vadv and others added 27 commits September 21, 2026 11:53
Keep NOTICE responses informational and restore built-in client RESET suppression. Avoid replaying Greengage startup metadata and remove duplicate cleanup warnings.

Add a disposable pinned Greengage cluster to the Docker Nix runner and CI, cover cleanup and RELOAD behavior with BDD, and update Russian and English documentation.
Track the actual frontend bundle through the existing directory and recursive file watches. Remove the nonexistent index.html dependency that made every Cargo invocation rebuild pg_doorman.
A query replaces the built-in cleanup commands, so it belongs to `always`
alone. Validate the effective mode and query of every pool instead of the
general block only: a pool that inherits a query and narrows the mode to
`adaptive` silently changes what the cleanup does. `off` keeps accepting an
inherited query, because a pool may raise another pool to `always`.

Document the pairing, the commands `adaptive` does not track, and refresh
the generated reference configs.
DISCARD ALL resets every GUC on the backend, including startup parameters
that never reported ParameterStatus, so the remembered startup values went
stale for the next checkout. Drop them from the snapshot instead of
re-arming cleanup: a configured query now exists only in the `always` mode,
which runs it regardless of the tracked state, so the re-arm could not
change the outcome.

Cover mode inheritance and the built-in cleanup commands counted in the
PostgreSQL log.
Documentation said the same pairing rule three times in different words and
listed the adaptive limitations as a pile of exceptions. State the rule once,
separate what adaptive tracks from what it does not, and give a Greengage
query that actually clears temporary tables on segments: DISCARD TEMP does
that, the previous example omitted it and then apologised for the gap.

Comments in the server, stats and metrics code explained the line below them
or repeated the metric help text. Keep only the notes that carry a fact the
code does not show: startup parameters a backend never reports back, why a
maintenance failure must not replace a completed result, why a partially reset
backend is never returned to the pool.
…etry

The TLS retry replaces the plain attempt, and a protocol-level plain failure
logs nothing on its own, so a backend that never started leaves only the retry
error behind: "tls required but server does not support tls" against a Postgres
with ssl=off, which says nothing about why the plain connection was rejected.
Report the original error in both retry lines: local backend and fallback
candidate.
…sentences

pg_doorman re-prepares its own cached prepared statements after a cleanup
query, so only client-side named PREPARE/EXECUTE break.
…er default

DISCARD ALL is PgBouncer's default server_reset_query, and PostgreSQL lists
the exact statement sequence it expands to.
…ncer

3.11.2 is released, so the feature belongs to a new section. The cleanup
query example is PgBouncer's default server_reset_query, which is the
reference our users compare against.
The protocol guards warned, mark_bad logged the same reason, and the
checkin_cleanup wrapper logged the outcome: three lines for one incident.
Keep mark_bad and the wrapper line.
The counter was only mentioned in the changelog. Spell out what it counts
in each cleanup mode, that result="error" means a retired backend, and that
the database label is the pool name.
cleanup_legacy described neither the mode nor the work. The rollback runs
in every mode, the RESET sequence only in adaptive, so the call site now
computes needs_rollback and needs_session_reset and passes the latter in.
off rolls back a transaction the client left open and sends nothing else.
adaptive adds the built-in reset for a changed session, including the
DEALLOCATE ALL that re-synchronizes a Parse the client never flushed.
@vadv
vadv force-pushed the feat/configurable-backend-cleanup branch 3 times, most recently from 964a7b7 to f9b6e1a Compare October 7, 2026 06:58
Add cleanup_server_connections modes off, adaptive, always and the
cleanup_server_query setting. Both work in [general] and per pool.

- adaptive (default). Built-in cleanup SQL runs only after a tracked
  session state change.
- always. cleanup_server_query runs on every checkin of a backend that
  served a client. The query is required.
- off. No session cleanup. ROLLBACK for an open transaction still runs.

Legacy true and false mean adaptive and off.

Adaptive detection covers SET, DECLARE cursors and the prepared statement
cache. This commit adds SQL PREPARE, LISTEN and CREATE TEMP TABLE:

- SQL PREPARE -> DEALLOCATE ALL. SQL PREPARE shares the server-side
  statement namespace with an extended-protocol Parse. Without tracking,
  the next client on the backend failed with 42P05.
- LISTEN -> UNLISTEN *. A pooled backend's socket is not read, so queued
  notifications reached the client that checked the backend out next.
- CREATE TEMP TABLE -> DISCARD TEMP. The CREATE TABLE tag does not
  distinguish temp from permanent tables; DISCARD TEMP is a no-op for
  permanent objects. SELECT INTO TEMP and CREATE TEMP TABLE AS SELECT
  complete with the inner query's tag on current PostgreSQL and stay
  untracked.
- Every cleanup batch releases advisory locks with
  pg_advisory_unlock_all().

A failed cleanup retires the backend. Responses already sent to the
client stay delivered. Client RESET and DISCARD ALL still suppress
built-in cleanup in adaptive.

Tests: BDD scenarios cover the leaks before the fix, the suppression
rules, async boundaries, observability metrics and a real Greengage
cluster. Docs: pool reference, concept pages, config examples and the
changelog.
@vadv
vadv force-pushed the feat/configurable-backend-cleanup branch from f9b6e1a to 8d4b541 Compare October 7, 2026 07:00

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant