diff --git a/.github/workflows/bdd-tests.yml b/.github/workflows/bdd-tests.yml index 63255c1d7..7c86a33c7 100644 --- a/.github/workflows/bdd-tests.yml +++ b/.github/workflows/bdd-tests.yml @@ -365,6 +365,7 @@ jobs: - { name: "Rust part 1", cargo: "test --test bdd -- --tags @rust-1" } - { name: "Rust part 2", cargo: "test --test bdd -- --tags @rust-2" } - { name: "Rust part 3", cargo: "test --test bdd -- --tags @rust-3" } + - { name: "Greengage 7 cleanup", cargo: "test --test bdd -- --tags @greengage" } - { name: "Rust part 4 (sleep-heavy lifecycle)", cargo: "test --test bdd -- --tags @rust-4" } - { name: "Binary upgrade graceful shutdown", cargo: "test --test bdd -- --tags @binary-upgrade-grac-shutdown" } - { name: "Fuzz", cargo: "test --test bdd -- --tags @fuzz" } @@ -431,7 +432,18 @@ jobs: retry_wait_seconds: 10 command: docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.prepare-tests.outputs.image-tag }} + - name: Run Greengage cleanup BDD + if: matrix.suite.name == 'Greengage 7 cleanup' + env: + REPO: ${{ github.repository }} + IMAGE_TAG: ${{ needs.prepare-tests.outputs.image-tag }} + CARGO_HOME: /workspace/.cargo + # prebuild-bdd populated this cache; avoid bridge-network DNS during tests. + CARGO_NET_OFFLINE: "true" + run: tests/nix/run-tests.sh greengage + - name: Run BDD suite (${{ matrix.suite.name }}) + if: matrix.suite.name != 'Greengage 7 cleanup' # `--network=host` is the load-bearing flag. With the default # bridge network, the container inherits DNS from the docker # daemon, which on a GitHub-hosted runner does not always diff --git a/.github/workflows/launchpad-publish.yaml b/.github/workflows/launchpad-publish.yaml index 3463693ff..e292ca461 100644 --- a/.github/workflows/launchpad-publish.yaml +++ b/.github/workflows/launchpad-publish.yaml @@ -1,7 +1,6 @@ name: Publish to Launchpad PPA on: - pull_request: release: types: [published] diff --git a/build.rs b/build.rs index c969ff1ab..cb0812968 100644 --- a/build.rs +++ b/build.rs @@ -11,7 +11,6 @@ use std::path::Path; fn main() { let dist = Path::new("frontend/dist"); println!("cargo:rerun-if-changed=frontend/dist"); - println!("cargo:rerun-if-changed=frontend/dist/index.html"); walk(dist); } diff --git a/documentation/en/src/changelog.md b/documentation/en/src/changelog.md index 37bf8159b..c2856df20 100644 --- a/documentation/en/src/changelog.md +++ b/documentation/en/src/changelog.md @@ -1,5 +1,51 @@ # Changelog +### 3.12.0 + +#### Configurable backend cleanup: `cleanup_server_connections` modes and `cleanup_server_query` + +`cleanup_server_connections` now accepts `off`, `adaptive` or `always`, in `[general]` and per pool. +Legacy `false` and `true` keep working and mean `off` and `adaptive`. + +- `adaptive` (default): built-in cleanup, sent only when pg_doorman saw the 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 of an unfinished transaction still runs. + +`cleanup_server_query` replaces the built-in `RESET ROLE` / `RESET ALL` / `DEALLOCATE ALL` / +`CLOSE ALL` sequence with your own SQL. A good example is PgBouncer's default `server_reset_query`, +`DISCARD ALL`. Validation checks the pair as a whole, with inherited values where a pool sets nothing. +`always` without a query is an error. A query with an effective `adaptive` mode is an error. + +```toml +[general] +cleanup_server_connections = "always" +cleanup_server_query = "DISCARD ALL" +``` + +A failed cleanup retires the backend instead of returning it to the pool with unknown session state. +In `always` the configured query runs on every checkin. A client-side `RESET` or `DISCARD ALL` does not +suppress it. In `adaptive` such a client reset suppresses the built-in cleanup. +A cleanup query like `DISCARD ALL` also removes prepared statements: they are re-parsed after every +checkin. Workloads that rely on prepared statements should stay in `adaptive`. +`pg_doorman_server_cleanup_total` counts cleanup attempts per pool with `result="ok"` or +`result="error"`. Pool-level `cleanup_server_connections` became optional in the config dump, so +`SHOW` and config dumps omit the field when the pool does not set it. + +Adaptive detection covers more session state: + +- SQL `PREPARE` → `DEALLOCATE ALL` at checkin. SQL `PREPARE` and an extended-protocol `Parse` share one + server-side statement namespace; previously the next client on the same backend could fail with + `42P05 duplicate_prepared_statement`. +- `LISTEN` → `UNLISTEN *` at checkin. A pooled backend's socket is not read, so queued notifications + were delivered to whichever client checked the backend out next. +- `CREATE TEMP TABLE` → `DISCARD TEMP` at checkin. `SELECT ... INTO TEMP` and `CREATE TEMP TABLE AS SELECT` + complete with the inner query's tag and stay untracked. In transaction mode the backend returns to the pool + after every query. A temp table no longer survives the same client's next query; keep a transaction open or + use session mode to keep it longer. +- Every cleanup batch releases advisory locks with `pg_advisory_unlock_all()`. Previously a session-level + advisory lock outlived the client that took it. + ### 3.11.2 #### Talos routes `s2i|`-prefixed clients through a service pool diff --git a/documentation/en/src/comparison.md b/documentation/en/src/comparison.md index ef8655862..48e9d7185 100644 --- a/documentation/en/src/comparison.md +++ b/documentation/en/src/comparison.md @@ -1,8 +1,6 @@ # PgDoorman vs PgBouncer vs Odyssey -Side-by-side feature matrix for choosing a PostgreSQL connection pooler. Every PgBouncer claim is anchored to its [config reference](https://www.pgbouncer.org/config.html) and [changelog](https://www.pgbouncer.org/changelog.html); every Odyssey claim is anchored to the project's [docs](https://github.com/yandex/odyssey/tree/master/docs). - -PgCat is intentionally omitted: its design centre is sharding/load-balancing rather than drop-in replacement of PgBouncer, so a row-by-row comparison is misleading. See the [PgCat repo](https://github.com/postgresml/pgcat) if you need horizontal sharding. +Side-by-side feature matrix for choosing a PostgreSQL connection pooler: PgDoorman, PgBouncer and Odyssey. For benchmark numbers, see [Benchmarks](benchmarks.md). @@ -73,7 +71,7 @@ See [Patroni-assisted fallback](tutorials/patroni-assisted-fallback.md), [`patro | `min_pool_size` (warm connections) | Yes | No | Yes | | Prepared statements in transaction mode | Yes (named and anonymous, two-level cache, query interner) | Yes (named, since 1.21, `max_prepared_statements`) | Yes (named, `pool_reserve_prepared_statement`) | | Anonymous `Parse` cache for performance | Yes (`DOORMAN_N`, reused across clients in a pool) | No (anonymous `Parse` passes through unchanged) | No (named statements required) | -| Smart cleanup on checkin (skip `DEALLOCATE ALL` if cache untouched) | Yes (mutation-tracking `RESET ALL` / `DEALLOCATE ALL` on demand) | No (always `DISCARD ALL` if `server_reset_query` set) | Yes (auto) | +| Session cleanup | [`adaptive` (default), `always`, `off`](reference/pool.md#cleanup_server_connections); custom SQL | `server_reset_query`; transaction mode requires [`server_reset_query_always=1`](https://www.pgbouncer.org/config.html#server_reset_query_always) | [`pool_discard` / `pool_smart_discard`](https://github.com/yandex/odyssey/blob/master/docs/configuration/rules.md#pool_discard) | | LISTEN / NOTIFY pinning in transaction mode | No | No | Experimental | | Cross-rule connection cap (`shared_pool`) | No | No | Yes (since 1.5.1) | | `PAUSE` / `RESUME` / `RECONNECT` admin commands | Yes | Yes | Yes (since 1.4.1) | diff --git a/documentation/en/src/concepts/pool-modes.md b/documentation/en/src/concepts/pool-modes.md index dcb49bf88..4a7d8c1f8 100644 --- a/documentation/en/src/concepts/pool-modes.md +++ b/documentation/en/src/concepts/pool-modes.md @@ -2,7 +2,7 @@ PgDoorman supports two pool modes: `transaction` and `session`. Set per pool, with optional per-user override. -There is no `statement` mode. Statement pooling rotates the backend after every statement, which forces clients to give up multi-statement transactions and breaks the prepared-statement protocol entirely; PgDoorman invests its tuning (prepared-statement cache, direct handoff, strict-FIFO scheduling) in transaction mode instead. PgBouncer keeps `statement` mode for backward compatibility; Odyssey omits it. +There is no `statement` mode. ## Transaction mode (recommended) @@ -12,23 +12,11 @@ pools: pool_mode: "transaction" ``` -A backend connection is held for the duration of a transaction, then returned to the pool on `COMMIT`, `ROLLBACK`, or implicit completion. +Supports: -This is the mode that delivers PgDoorman's connection efficiency: a `pool_size` of 40 can serve thousands of clients as long as transactions are short. - -What works in transaction mode (where most poolers fail): - -- Prepared statements. PgDoorman caches them per-pool, remaps statement names across backend connections, and replays preparation transparently. Drivers that pin to `unnamed` statement (Go pgx, .NET Npgsql, Python asyncpg) work without configuration. -- Pipelined batches and async `Flush` flow. -- Cancel requests over TLS. -- `LISTEN` / `NOTIFY` — but only inside a transaction. A `LISTEN` issued and then committed releases the backend, and any notifications delivered to it after that go to whichever client checks it out next, not to the original `LISTEN`-er. PgBouncer behaves the same way; if you need cross-transaction `LISTEN`, use session mode for that client. - -What does **not** work in transaction mode: - -- `SET` and `RESET` outside a transaction. Use session mode for clients that rely on session-level GUC changes (`SET TIME ZONE`, `SET search_path` once per connection). -- Advisory locks held across transactions. Use session mode. -- Cursors held outside transactions (`WITH HOLD`). Use session mode. -- `SET LOCAL` works as expected — it is transaction-scoped. +- Named and anonymous prepared statements. +- Pipelined batches and asynchronous `Flush`. +- Query cancellation over TLS. ## Session mode @@ -38,16 +26,11 @@ pools: pool_mode: "session" ``` -A backend connection is held for the duration of the client session. Returned to the pool only when the client disconnects. - -Use this when: - -- The application uses session-scoped state (`SET search_path`, `SET TIME ZONE`). -- The application uses `WITH HOLD` cursors. -- The application uses advisory locks across transactions. -- You are migrating an unmodified PgBouncer deployment that was using session mode and you want a like-for-like swap. +Use for clients that need: -In session mode, `pool_size` is effectively the maximum number of concurrent clients. Sizing matches PostgreSQL's `max_connections` minus reserves. +- Session parameters (`SET search_path`, `SET TIME ZONE`). +- `LISTEN` subscriptions. +- `WITH HOLD` cursors and advisory locks held across transactions. ## Per-user override @@ -71,26 +54,25 @@ Useful when one user (operations tooling, migrations) needs session semantics bu ## Cleanup on checkin -Cleanup in transaction mode is **mutation-tracked**, not unconditional. PgDoorman watches each transaction for `SET`, `PREPARE`, and `DECLARE CURSOR`, and only when the backend returns to the pool with one of those flags set does it issue `RESET ALL`, `DEALLOCATE ALL`, or `CLOSE ALL` respectively. A read-only transaction skips cleanup entirely — that's a measurable win on hot OLTP paths. +The default is `cleanup_server_connections: adaptive` without `cleanup_server_query`: pg_doorman cleans a session only when it sees the session state change, so the pooler's cached prepared statements survive a checkin. -What gets reset when a flag fires: +The built-in cleanup tracks five kinds of session state and answers with cleanup SQL: -- `SET` flag → `RESET ALL` drops session-level GUCs and runs `pg_advisory_unlock_all` implicitly. -- `PREPARE` flag → `DEALLOCATE ALL` drops PostgreSQL-side prepared statements that the driver named explicitly. PgDoorman's own prepared-statement cache survives the reset because it is keyed by query text, not by backend name. -- `DECLARE CURSOR` flag → `CLOSE ALL` drops cursors. +- `SET` → `RESET ALL`. +- `DECLARE` cursor → `CLOSE ALL`. +- SQL `PREPARE` → `DEALLOCATE ALL`. SQL `PREPARE` and an extended-protocol `Parse` share one statement namespace. +- `LISTEN` → `UNLISTEN *`. +- `CREATE TEMP TABLE` → `DISCARD TEMP`. -`DEALLOCATE ALL` and `DISCARD ALL` issued by the client clear that client's prepared-statement cache (so the next `Parse` registers anew). The pool-level shared cache is not affected; other clients keep their entries. +Every cleanup batch also releases advisory locks with `pg_advisory_unlock_all()`. -To opt out of cleanup entirely (for performance, in tightly-controlled deployments): +Not tracked: `SELECT ... INTO TEMP` and `CREATE TEMP TABLE AS SELECT` — they complete with the inner query's tag. Temporary objects created inside functions carry no tag. Use `always` with suitable SQL to release these. -```yaml -pools: - mydb: - pool_mode: "transaction" - cleanup_server_connections: false -``` +In transaction pooling the backend returns to the pool after every query. A temporary table lives for one transaction; keep a transaction open across the statements that use it. + +`cleanup_server_query` replaces the built-in cleanup and requires `always`: your SQL runs on every checkin of a backend that served a client. It is incompatible with `adaptive`, and validation rejects that pairing. The mode costs the server one extra query per checkin. With `DISCARD ALL`, prepared statements are also removed and re-parsed after every checkin. `off` disables session cleanup. Open transactions are rolled back in every mode. -Only do this if you are sure your application never leaks session state. The mutation-tracked default is already cheap when no mutation happened, so the opt-out is rarely worth the risk. +See the [pool reference](../reference/pool.md#cleanup_server_connections) for the full list of tracked statements, limitations, and PostgreSQL and Greengage examples. ## Reference diff --git a/documentation/en/src/tutorials/contributing.md b/documentation/en/src/tutorials/contributing.md index bb7d148fb..5ed7b07a8 100644 --- a/documentation/en/src/tutorials/contributing.md +++ b/documentation/en/src/tutorials/contributing.md @@ -93,6 +93,20 @@ make test-bdd TAGS=@admin-commands make shell ``` +### Greengage + +```bash +make test-greengage +``` + +This starts Greengage 7.5.0 with a coordinator and two primary segments, +then runs `@greengage` in the same Nix environment as the PostgreSQL tests. +The Greengage image is pinned by digest in `tests/nix/greengage.sh`; +ARM64 hosts use amd64 emulation. Each scenario gets a separate database, +and the container is removed after the run. These tests also run in CI. + +Select a scenario with `make test-greengage TAGS=@greengage-cleanup-built-in`. + ### Debug Mode Enable debug output with the `DEBUG=1` environment variable: @@ -125,6 +139,7 @@ This is useful when: | `@java` | Java client tests (JDBC) | | `@php` | PHP client tests (PDO) | | `@rust` | Rust protocol-level tests | +| `@greengage` | Connection cleanup on a real Greengage cluster | | `@auth-query` | Auth query authentication tests | | `@copy-protocol` | COPY protocol tests | | `@cancel` | Query cancellation tests | diff --git a/documentation/ru/src/comparison.md b/documentation/ru/src/comparison.md index f0fec6ccc..ebb2234be 100644 --- a/documentation/ru/src/comparison.md +++ b/documentation/ru/src/comparison.md @@ -1,8 +1,6 @@ # PgDoorman vs PgBouncer vs Odyssey -Сравнительная матрица фич для выбора пулера соединений PostgreSQL. Каждое утверждение про PgBouncer привязано к [config reference](https://www.pgbouncer.org/config.html) и [changelog](https://www.pgbouncer.org/changelog.html); каждое утверждение про Odyssey — к [docs](https://github.com/yandex/odyssey/tree/master/docs) проекта. - -PgCat намеренно опущен: у него центр тяжести — шардинг и балансировка, а не drop-in замена PgBouncer, поэтому построчное сравнение вводит в заблуждение. Если нужен горизонтальный шардинг, см. [репозиторий PgCat](https://github.com/postgresml/pgcat). +Сравнение возможностей пулеров PostgreSQL: pg_doorman, PgBouncer и Odyssey. Цифры из бенчмарков — [Бенчмарки](benchmarks.md). @@ -73,7 +71,7 @@ PgCat намеренно опущен: у него центр тяжести — | `min_pool_size` (warm connections) | Да | Нет | Да | | Prepared statements в transaction mode | Да (именованные и анонимные, двухуровневый кеш, query interner) | Да (именованные, с 1.21, `max_prepared_statements`) | Да (именованные, `pool_reserve_prepared_statement`) | | Кеш анонимного `Parse` для производительности | Да (`DOORMAN_N`, переиспользование между клиентами пула) | Нет (анонимный `Parse` проходит без изменений) | Нет (требуются именованные prepared statements) | -| Умная очистка при возврате соединения (пропустить `DEALLOCATE ALL`, если кеш не менялся) | Да (`RESET ALL` / `DEALLOCATE ALL` по факту мутаций) | Нет (всегда `DISCARD ALL`, если задан `server_reset_query`) | Да (auto) | +| Очистка сессии | [`adaptive` (по умолчанию), `always`, `off`](reference/pool.md#cleanup_server_connections); свой SQL | `server_reset_query`; в transaction требуется [`server_reset_query_always=1`](https://www.pgbouncer.org/config.html#server_reset_query_always) | [`pool_discard` / `pool_smart_discard`](https://github.com/yandex/odyssey/blob/master/docs/configuration/rules.md#pool_discard) | | LISTEN / NOTIFY pinning в transaction mode | Нет | Нет | Экспериментально | | Cross-rule connection cap (`shared_pool`) | Нет | Нет | Да (с 1.5.1) | | Команды администратора `PAUSE` / `RESUME` / `RECONNECT` | Да | Да | Да (с 1.4.1) | diff --git a/documentation/ru/src/concepts/pool-modes.md b/documentation/ru/src/concepts/pool-modes.md index 3027a7e7f..34583d837 100644 --- a/documentation/ru/src/concepts/pool-modes.md +++ b/documentation/ru/src/concepts/pool-modes.md @@ -2,7 +2,7 @@ pg_doorman поддерживает два режима пула: `transaction` и `session`. Режим задаётся для пула, при необходимости переопределяется для конкретного пользователя. -Режима `statement` нет. В statement-пулинге серверное соединение ротируется после каждого оператора — это вынуждает клиентов отказаться от мульти-statement транзакций и полностью ломает протокол prepared statements. Все оптимизации pg_doorman (кеш prepared statements, прямая передача, строгий FIFO-планировщик) рассчитаны на транзакционный режим. PgBouncer оставляет `statement` для обратной совместимости; Odyssey его не реализует. +Режима `statement` нет. ## Транзакционный режим (рекомендуется) @@ -12,23 +12,11 @@ pools: pool_mode: "transaction" ``` -Backend-соединение удерживается в течение транзакции и возвращается в пул при `COMMIT`, `ROLLBACK` или неявном завершении. +Поддерживаются: -Именно этот режим даёт ту самую эффективность соединений, ради которой существует pg_doorman: `pool_size` равный 40 обслуживает тысячи клиентов, пока транзакции остаются короткими. - -Что работает в транзакционном режиме (там, где большинство пулеров проваливаются): - -- Prepared statements. pg_doorman кеширует их в рамках пула, переименовывает имена statement между backend-соединениями и прозрачно повторно готовит запрос. Драйверы, привязанные к `unnamed`-statement (Go pgx, .NET Npgsql, Python asyncpg), работают без настройки. -- Pipelined-батчи и асинхронный поток `Flush`. -- Cancel-запросы поверх TLS. -- `LISTEN` / `NOTIFY` — но только внутри транзакции. `LISTEN` после `COMMIT` отпускает backend, и любые уведомления, доставленные ему после, попадут к тому клиенту, который следующим заберёт это соединение, а не к исходному `LISTEN`-еру. PgBouncer ведёт себя так же; если вам нужен `LISTEN` через несколько транзакций, для этого клиента используйте сессионный режим. - -Что в транзакционном режиме **не работает**: - -- `SET` и `RESET` вне транзакции. Используйте сессионный режим для клиентов, опирающихся на изменение GUC уровня сессии (`SET TIME ZONE`, `SET search_path` один раз на соединение). -- advisory-блокировки, удерживаемые между транзакциями. Используйте сессионный режим. -- курсоры, удерживаемые вне транзакций (`WITH HOLD`). Используйте сессионный режим. -- `SET LOCAL` работает как ожидается — он ограничен транзакцией. +- Именованные и анонимные подготовленные запросы. +- Pipeline и асинхронный `Flush`. +- Отмена запросов через TLS. ## Сессионный режим @@ -38,16 +26,11 @@ pools: pool_mode: "session" ``` -Backend-соединение удерживается в течение клиентской сессии. В пул возвращается только при отключении клиента. - -Используйте, когда: - -- Приложение использует состояние уровня сессии (`SET search_path`, `SET TIME ZONE`). -- Приложение использует курсоры WITH HOLD. -- Приложение использует advisory-блокировки между транзакциями. -- Вы переезжаете с немодифицированного PgBouncer-развёртывания, работавшего в сессионном режиме, и хотите подмену один-в-один. +Используйте для клиентов, которым нужны: -В сессионном режиме `pool_size` фактически равен максимальному числу одновременных клиентов. Размер пула подбирается так, чтобы соответствовать `max_connections` PostgreSQL минус резервы. +- Параметры сессии (`SET search_path`, `SET TIME ZONE`). +- Подписки `LISTEN`. +- Курсоры `WITH HOLD` и advisory-блокировки между транзакциями. ## Переопределение для конкретного пользователя @@ -71,26 +54,25 @@ pools: ## Очистка при возврате в пул -Очистка в транзакционном режиме **отслеживает мутации**, а не выполняется безусловно. pg_doorman следит за каждой транзакцией на предмет `SET`, `PREPARE` и `DECLARE CURSOR`, и только когда backend уходит в пул с одним из этих флагов, отправляет соответственно `RESET ALL`, `DEALLOCATE ALL` или `CLOSE ALL`. Транзакция только на чтение пропускает очистку целиком — это измеримый выигрыш на горячих OLTP-путях. +По умолчанию включён `cleanup_server_connections: adaptive` без `cleanup_server_query`: pg_doorman очищает сессию только когда видит её изменение, поэтому кешированные prepared statements пулера переживают возврат соединения в пул. -Что сбрасывается, когда сработал флаг: +Встроенная очистка отслеживает пять видов состояния сессии и отвечает очисткой: -- Флаг `SET` → `RESET ALL` сбрасывает session-level GUCs и неявно вызывает `pg_advisory_unlock_all`. -- Флаг `PREPARE` → `DEALLOCATE ALL` удаляет PostgreSQL-side prepared statements, которые драйвер именовал явно. Собственный кеш prepared statements pg_doorman сохраняется после сброса: он индексируется текстом запроса, а не backend-именем. -- Флаг `DECLARE CURSOR` → `CLOSE ALL` закрывает курсоры. +- `SET` → `RESET ALL`. +- Курсор `DECLARE` → `CLOSE ALL`. +- SQL `PREPARE` → `DEALLOCATE ALL`. SQL `PREPARE` и extended-протокольный `Parse` делят одно пространство имён statements на сервере. +- `LISTEN` → `UNLISTEN *`. +- `CREATE TEMP TABLE` → `DISCARD TEMP`. -`DEALLOCATE ALL` и `DISCARD ALL` со стороны клиента очищают prepared-statement-кеш именно этого клиента (следующий `Parse` зарегистрируется заново). Pool-level shared cache не затрагивается; у других клиентов их записи сохраняются. +Каждая пачка очистки также снимает advisory-блокировки через `pg_advisory_unlock_all()`. -Полностью отключить очистку (ради производительности в жёстко контролируемых развёртываниях): +Не отслеживаются `SELECT ... INTO TEMP` и `CREATE TEMP TABLE AS SELECT`: они отвечают тегом внутреннего запроса. Временные объекты внутри функций тега не имеют. Для них используйте `always` с подходящим SQL. -```yaml -pools: - mydb: - pool_mode: "transaction" - cleanup_server_connections: false -``` +В транзакционном режиме соединение возвращается в пул после каждого запроса. Временная таблица живёт одну транзакцию: держите транзакцию открытой для всех операторов, которые её используют. + +`cleanup_server_query` заменяет встроенную очистку и требует `always`: собственный SQL выполняется при каждом возврате использованного соединения. С `adaptive` он несовместим, и такая конфигурация отклоняется при проверке. Режим дороже для сервера: запрос уходит на каждый возврат. С `DISCARD ALL` prepared statements ещё и пере-парсиваются после каждого возврата. `off` отключает очистку сессии. Незакрытые транзакции откатываются независимо от режима. -Делайте так только если уверены, что приложение никогда не оставляет состояние сессии. Очистка по умолчанию уже дёшева на транзакциях без мутаций, поэтому отключение редко стоит риска. +Полный список отслеживаемых операторов, ограничения и примеры для PostgreSQL и Greengage см. в [справочнике](../reference/pool.md#cleanup_server_connections). ## Справочник diff --git a/documentation/ru/src/reference/general.md b/documentation/ru/src/reference/general.md index cc05afa01..14afc23de 100644 --- a/documentation/ru/src/reference/general.md +++ b/documentation/ru/src/reference/general.md @@ -360,6 +360,18 @@ TCP backlog для входящих соединений. При значени По умолчанию: `false`. +### cleanup_server_connections + +Режим очистки сессии по умолчанию: `off`, `adaptive` или `always`. Применяется ко всем пулам, если пул не переопределил его. +Старые значения `false` и `true` означают `off` и `adaptive`. +Выбор режима и ограничения описаны в [настройках пула](pool.md#cleanup_server_connections). + +### cleanup_server_query + +SQL очистки сессии для режима `always`. По умолчанию не задан. +Пулы наследуют его из `general`. Собственный запрос требует `cleanup_server_connections = always`. +Примеры и требования к SQL описаны в [настройках пула](pool.md#cleanup_server_query). + ### sync_server_parameters В транзакционном режиме разные транзакции одного клиента могут выполняться на разных серверных diff --git a/documentation/ru/src/reference/pool.md b/documentation/ru/src/reference/pool.md index b6e93334a..f91df4210 100644 --- a/documentation/ru/src/reference/pool.md +++ b/documentation/ru/src/reference/pool.md @@ -62,14 +62,76 @@ ### cleanup_server_connections -Сбрасывать ли состояние сессии при возврате соединения в пул. -Когда параметр включён и сессия была изменена, pg_doorman отправляет: `RESET ROLE`, плюс при необходимости -`RESET ALL` (если использовался SET), `DEALLOCATE ALL` (если использовался PREPARE), `CLOSE ALL` -(если открывались курсоры). Замечание: `ROLLBACK` для открытых транзакций выполняется всегда, независимо -от этой настройки. Отключайте только если ваше приложение никогда не использует SET, prepared statements -или курсоры и вы хотите сэкономить roundtrip на очистке. - -По умолчанию: `true`. +Режим очистки сессии при возврате соединения в пул: + +- `adaptive` (по умолчанию): встроенная очистка, включается по отслеженным изменениям состояния сессии. Обычные запросы, в том числе с кешируемыми prepared statements, не добавляют лишнего SQL. `cleanup_server_query` в этом режиме — ошибка конфигурации. +- `always`: `cleanup_server_query` выполняется при каждом возврате соединения, которое обслуживало клиента. Запрос обязателен. +- `off`: очистка не выполняется. `ROLLBACK` незавершённой транзакции по-прежнему выполняется. + +Встроенная очистка отправляет `RESET ROLE`, а при необходимости `RESET ALL`, `DEALLOCATE ALL`, `CLOSE ALL`, +`UNLISTEN *` и `DISCARD TEMP`. Если клиент сам выполнил `RESET ALL` (например, pgx), повторный `RESET ALL` +не отправляется. +Каждая пачка очистки снимает и advisory-блокировки через `pg_advisory_unlock_all()`. +Если сервер не знает эту функцию, пачка падает и бэкенд выводится из пула. + +Отслеживаются (оператор → очистка): + +- `SET` → `RESET ALL`. +- Курсоры `DECLARE` → `CLOSE ALL`. +- SQL `PREPARE` и рассогласование кеша prepared statements с сервером → `DEALLOCATE ALL`. SQL `PREPARE` и extended-протокольный `Parse` живут в одном пространстве имён на сервере. Без очистки следующий клиент на этом соединении получает `42P05 duplicate_prepared_statement`. +- `LISTEN` → `UNLISTEN *`. Соединение в пуле никто не читает: накопленные `NotificationResponse` достались бы следующему клиенту. Клиентский `UNLISTEN` не глушит очистку: один тег покрывает и `UNLISTEN ch`, и `UNLISTEN *`. Частичная отписка не доказывает, что подписок не осталось. +- `CREATE TEMP TABLE` → `DISCARD TEMP`. Тег `CREATE TABLE` не различает временную и постоянную таблицу: постоянный `CREATE TABLE` тоже приводит к `DISCARD TEMP`, для постоянных объектов он безвреден. На Greengage `DISCARD TEMP` удаляет временные таблицы и с координатора, и с сегментов. + В транзакционном режиме соединение возвращается в пул после каждого запроса. Временная таблица живёт одну транзакцию: держите транзакцию открытой для всех операторов, которые её используют, или используйте сессионный режим. + +Не отслеживаются: + +- `SELECT ... INTO TEMP` и `CREATE TEMP TABLE AS SELECT`: текущий PostgreSQL отвечает на них тегом внутреннего `SELECT`, неотличимым от обычного чтения. На форках, где эти выражения отвечают собственными тегами, очистка включается. Временные таблицы и последовательности, созданные внутри функций, тега не имеют вовсе. +- Advisory-блокировки: собственного тега нет, поэтому блокировка, взятая единственной операцией, очистку + не включает. Когда пачка уходит по другой причине, `pg_advisory_unlock_all()` в ней снимает и их. + +Не отслеживаемые ресурсы безусловно освобождает только `always` с подходящим SQL. Отслеживание по тегам +даёт и такие следствия: + +- `RESET` одного параметра и `RESET ALL` возвращают одинаковый тег, поэтому клиентский сброс одного параметра отменяет последующий `RESET ALL`, и другие изменённые параметры могут остаться установленными. +- В `always` клиентские `RESET` и `DISCARD ALL` не заменяют настроенный запрос. + +Режим и запрос наследуются из `general` раздельно, но проверяются вместе: `always` требует запрос, `adaptive` его не допускает. Старые значения `true` и `false` означают `adaptive` и `off`. +Изменение политики через `RELOAD` применяется к новым клиентским подключениям. + +### cleanup_server_query + +SQL вместо встроенной очистки. Выполняется при каждом возврате соединения, которое обслуживало клиента, и требует `cleanup_server_connections = always`: пул в `adaptive` с собственным или унаследованным запросом — ошибка конфигурации. В `off` запрос не используется. +По умолчанию пул наследует `general.cleanup_server_query`. + +Запрос должен сбрасывать параметры сессии и освобождать prepared statements, курсоры и другие ресурсы приложения. +Он не должен оставлять открытую транзакцию или переходить в COPY. +Ошибка выполнения или таймаут `connect_timeout` закрывают серверное соединение. + +Хороший пример — `cleanup_server_query = "DISCARD ALL"`, тот же запрос PgBouncer держит по умолчанию как +`server_reset_query`. В PostgreSQL `DISCARD ALL` равна набору `CLOSE ALL; SET SESSION AUTHORIZATION DEFAULT; +RESET ALL; DEALLOCATE ALL; UNLISTEN *; SELECT pg_advisory_unlock_all(); DISCARD PLANS; DISCARD SEQUENCES; +DISCARD TEMP`, поэтому она шире встроенной очистки: добавляет `SET SESSION AUTHORIZATION DEFAULT`, +`DISCARD PLANS` и `DISCARD SEQUENCES`. На Greengage у неё есть особенность: `DISCARD ALL` освобождает +временную таблицу на координаторе, но оставляет её на сегментах. Пример запроса для Greengage 7: + +```sql +CLOSE ALL; +SET SESSION AUTHORIZATION DEFAULT; +RESET ALL; +DEALLOCATE ALL; +UNLISTEN *; +SELECT pg_advisory_unlock_all(); +DISCARD PLANS; +DISCARD SEQUENCES; +DISCARD TEMP +``` + +`DISCARD TEMP` удаляет временные таблицы и с координатора, и с сегментов. Без него временные объекты и состояние последовательностей остаются в сессии. + +Цена для prepared statements. `DISCARD ALL` удаляет и prepared statements. В транзакционном режиме +statement парсится заново в каждой транзакции, которая его использует. В сессионном режиме statements +переиспользуются внутри одного подключения. Если нагрузка опирается на prepared statements, +используйте `adaptive`. ### scaling_warm_pool_ratio diff --git a/documentation/ru/src/reference/prometheus.md b/documentation/ru/src/reference/prometheus.md index ecd6698c8..0836a6433 100644 --- a/documentation/ru/src/reference/prometheus.md +++ b/documentation/ru/src/reference/prometheus.md @@ -114,6 +114,7 @@ pg_doorman экспортирует следующие метрики: | `pg_doorman_servers_prepared_misses` | Текущая сумма промахов prepared statements по активным бэкендам пула, с лейблами `user` и `database`. Gauge может уменьшаться при ротации бэкендов; для `rate()` используйте `pg_doorman_servers_prepared_misses_total`. | | `pg_doorman_servers_prepared_hits_total` | Накопительный счётчик попаданий в кеш prepared statements по всем бэкендам пула, с лейблами `user` и `database`. Используйте `rate()` для скорости попаданий. | | `pg_doorman_servers_prepared_misses_total` | Накопительный счётчик промахов prepared statements по всем бэкендам пула, с лейблами `user` и `database`. Устойчивая ненулевая скорость означает, что запросы часто готовятся заново или кеш `server_prepared_statements_cache_size` слишком мал. | +| `pg_doorman_server_cleanup_total` | Накопительный счётчик попыток очистки бэкенда, с лейблами `user`, `database` и `result` (`ok` или `error`). Растёт один раз на каждую попытку, в которой pg_doorman отправил бэкенду очисточный SQL: встроенную последовательность в `adaptive`, `cleanup_server_query` в `always` или `ROLLBACK` транзакции, которую клиент оставил незакрытой. В режиме `off` считается только этот `ROLLBACK`, поэтому ненулевая скорость там означает брошенные клиентом транзакции. `result="error"` считает попытки, после которых бэкенд выведен из пула: запрос очистки упал, не уложился в `connect_timeout` или оставил бэкенд не в Idle. Возвраты соединений, где очистки не потребовалось, не считаются. Лейбл `database` — имя пула. | ### Метрики клиентского кеша prepared statements diff --git a/documentation/ru/src/tutorials/contributing.md b/documentation/ru/src/tutorials/contributing.md index d58ad90ed..4e59312f7 100644 --- a/documentation/ru/src/tutorials/contributing.md +++ b/documentation/ru/src/tutorials/contributing.md @@ -93,6 +93,20 @@ make test-bdd TAGS=@admin-commands make shell ``` +### Greengage + +```bash +make test-greengage +``` + +Команда запускает Greengage 7.5.0 с координатором и двумя primary-сегментами, +затем выполняет `@greengage` в том же Nix-окружении, что и тесты PostgreSQL. +Образ Greengage закреплён по digest в `tests/nix/greengage.sh`; +на ARM64 используется эмуляция amd64. Каждый сценарий получает отдельную БД, +контейнер удаляется после прогона. Эти тесты также запускаются в CI. + +Для выбора сценария используйте `make test-greengage TAGS=@greengage-cleanup-built-in`. + ### Debug-режим Включается переменной окружения `DEBUG=1`: @@ -125,6 +139,7 @@ DEBUG=1 make test-bdd TAGS=@copy-protocol | `@java` | Тесты Java-клиентов (JDBC) | | `@php` | Тесты PHP-клиентов (PDO) | | `@rust` | Тесты на уровне протокола, написанные на Rust | +| `@greengage` | Очистка соединений на реальном кластере Greengage | | `@auth-query` | Тесты auth query authentication | | `@copy-protocol` | Тесты COPY-протокола | | `@cancel` | Тесты отмены запросов | diff --git a/pg_doorman.toml b/pg_doorman.toml index fba666c01..955466fe4 100644 --- a/pg_doorman.toml +++ b/pg_doorman.toml @@ -244,6 +244,17 @@ server_round_robin = false # Default: false sync_server_parameters = false +# Session cleanup mode for all pools: off, adaptive, or always. +# Adaptive runs cleanup SQL only when the session state changed. +# Always runs cleanup_server_query on every checkin. +# Legacy false/true mean off/adaptive. +# Default: "adaptive" +cleanup_server_connections = "adaptive" + +# Session cleanup SQL for the always mode. Not allowed with adaptive. +# Default: None (built-in adaptive cleanup) +# cleanup_server_query = "DISCARD ALL" + # DataRow messages larger than this threshold are streamed to the client in small chunks # instead of being buffered entirely in memory. Prevents memory spikes on large rows. # Default: 1048576 (1048576 bytes) @@ -558,11 +569,16 @@ pool_mode = "transaction" # Override global server_lifetime for this pool (in milliseconds). # server_lifetime = 300000 -# Reset session state (SET, prepared statements, cursors) when returning a connection to pool. -# ROLLBACK for open transactions is always executed regardless of this setting. -# Prevents state leaking between clients in transaction mode. -# Default: true -cleanup_server_connections = true +# Override general.cleanup_server_connections: off, adaptive, or always. +# Adaptive runs cleanup SQL only when the session state changed. +# Always runs cleanup_server_query on every checkin. Off does not disable ROLLBACK. +# Default: None (uses global setting) +# cleanup_server_connections = "adaptive" + +# Override general.cleanup_server_query with custom session cleanup SQL. +# Requires cleanup_server_connections = always; off does not use it. +# Default: None (uses global setting) +# cleanup_server_query = "DISCARD ALL" # Override global sync_server_parameters for this pool. # Default: None (uses global setting) diff --git a/pg_doorman.yaml b/pg_doorman.yaml index 1737c751a..279f7a3b2 100644 --- a/pg_doorman.yaml +++ b/pg_doorman.yaml @@ -284,6 +284,17 @@ general: # Default: false sync_server_parameters: false + # Session cleanup mode for all pools: off, adaptive, or always. + # Adaptive runs cleanup SQL only when the session state changed. + # Always runs cleanup_server_query on every checkin. + # Legacy false/true mean off/adaptive. + # Default: "adaptive" + cleanup_server_connections: "adaptive" + + # Session cleanup SQL for the always mode. Not allowed with adaptive. + # Default: None (built-in adaptive cleanup) + # cleanup_server_query: "DISCARD ALL" + # DataRow messages larger than this threshold are streamed to the client in small chunks # instead of being buffered entirely in memory. Prevents memory spikes on large rows. # Supports human-readable format: "1MB", "1M", or 1048576 (bytes) @@ -605,11 +616,16 @@ pools: # Override global server_lifetime for this pool (in milliseconds). # server_lifetime: 300000 - # Reset session state (SET, prepared statements, cursors) when returning a connection to pool. - # ROLLBACK for open transactions is always executed regardless of this setting. - # Prevents state leaking between clients in transaction mode. - # Default: true - cleanup_server_connections: true + # Override general.cleanup_server_connections: off, adaptive, or always. + # Adaptive runs cleanup SQL only when the session state changed. + # Always runs cleanup_server_query on every checkin. Off does not disable ROLLBACK. + # Default: None (uses global setting) + # cleanup_server_connections: "adaptive" + + # Override general.cleanup_server_query with custom session cleanup SQL. + # Requires cleanup_server_connections = always; off does not use it. + # Default: None (uses global setting) + # cleanup_server_query: "DISCARD ALL" # Override global sync_server_parameters for this pool. # Default: None (uses global setting) diff --git a/src/app/generate/annotated.rs b/src/app/generate/annotated.rs index c37ae1637..a3ebe2296 100644 --- a/src/app/generate/annotated.rs +++ b/src/app/generate/annotated.rs @@ -153,7 +153,8 @@ pub fn generate_reference_config(format: ConfigFormat, russian: bool) -> String connect_timeout: None, idle_timeout: None, server_lifetime: None, - cleanup_server_connections: true, + cleanup_server_connections: None, + cleanup_server_query: None, log_client_parameter_status_changes: false, application_name: None, prepared_statements_cache_size: None, @@ -850,6 +851,26 @@ fn write_general_section(w: &mut ConfigWriter, config: &Config) { ); w.blank(); + write_field_comment(w, fi, "general", "cleanup_server_connections"); + w.kv( + fi, + "cleanup_server_connections", + &serde_json::to_string(&g.cleanup_server_connections).unwrap(), + ); + w.blank(); + + write_field_comment(w, fi, "general", "cleanup_server_query"); + if let Some(query) = &g.cleanup_server_query { + w.kv( + fi, + "cleanup_server_query", + &serde_json::to_string(query).unwrap(), + ); + } else { + w.commented_kv(fi, "cleanup_server_query", &w.str_val("DISCARD ALL")); + } + w.blank(); + write_field_desc(w, fi, "general", "message_size_to_be_stream"); write_byte_size_value( w, @@ -1379,11 +1400,27 @@ fn write_single_pool(w: &mut ConfigWriter, pool_name: &str, pool: &Pool) { w.blank(); write_field_comment(w, fi, "pool", "cleanup_server_connections"); - w.kv( - fi, - "cleanup_server_connections", - &w.bool_val(pool.cleanup_server_connections), - ); + if let Some(mode) = pool.cleanup_server_connections { + w.kv( + fi, + "cleanup_server_connections", + &serde_json::to_string(&mode).unwrap(), + ); + } else { + w.commented_kv(fi, "cleanup_server_connections", &w.str_val("adaptive")); + } + w.blank(); + + write_field_comment(w, fi, "pool", "cleanup_server_query"); + if let Some(query) = &pool.cleanup_server_query { + w.kv( + fi, + "cleanup_server_query", + &serde_json::to_string(query).unwrap(), + ); + } else { + w.commented_kv(fi, "cleanup_server_query", &w.str_val("DISCARD ALL")); + } w.blank(); write_field_comment(w, fi, "pool", "sync_server_parameters"); @@ -2088,6 +2125,43 @@ fn write_byte_size_value( mod tests { use super::*; + #[test] + fn cleanup_policy_roundtrips_both_formats() { + let mut config = Config::default(); + config.general.cleanup_server_connections = crate::config::CleanupMode::Always; + config.general.cleanup_server_query = Some("DISCARD ALL".into()); + let pool = Pool { + cleanup_server_connections: Some(crate::config::CleanupMode::Off), + cleanup_server_query: Some("RESET ALL;\nSELECT 'quoted \"value\"';".into()), + users: vec![User::default()], + ..Pool::default() + }; + config.pools.insert("test".into(), pool); + for format in [ConfigFormat::Toml, ConfigFormat::Yaml] { + let text = generate_annotated_config(&config, format, false); + let parsed: Config = match format { + ConfigFormat::Toml => toml::from_str(&text).unwrap(), + ConfigFormat::Yaml => serde_yaml::from_str(&text).unwrap(), + }; + assert_eq!( + parsed.general.cleanup_server_connections, + config.general.cleanup_server_connections + ); + assert_eq!( + parsed.pools["test"].cleanup_server_connections, + config.pools["test"].cleanup_server_connections + ); + assert_eq!( + parsed.general.cleanup_server_query, + config.general.cleanup_server_query + ); + assert_eq!( + parsed.pools["test"].cleanup_server_query, + config.pools["test"].cleanup_server_query + ); + } + } + #[test] fn test_fields_yaml_parses() { // Force parsing of fields.yaml and verify it doesn't panic diff --git a/src/app/generate/docs.rs b/src/app/generate/docs.rs index 86f094cb5..d9c042197 100644 --- a/src/app/generate/docs.rs +++ b/src/app/generate/docs.rs @@ -230,6 +230,8 @@ fn write_general_fields(out: &mut String, f: &FieldsData) { "server_idle_check_timeout", "server_round_robin", "sync_server_parameters", + "cleanup_server_connections", + "cleanup_server_query", "tcp_so_linger", "tcp_no_delay", "tcp_keepalives_count", @@ -286,6 +288,7 @@ fn write_pool_fields(out: &mut String, f: &FieldsData) { "pool_mode", "log_client_parameter_status_changes", "cleanup_server_connections", + "cleanup_server_query", "scaling_warm_pool_ratio", "scaling_fast_retries", "max_db_connections", @@ -490,7 +493,8 @@ fn write_prometheus_metrics_section(out: &mut String) { let _ = writeln!(out, "| `pg_doorman_servers_prepared_hits` | Live aggregate of prepared-statement cache hits across currently active backends of each pool, by user and database. This gauge can decrease when backends rotate; use `pg_doorman_servers_prepared_hits_total` for rates. |"); let _ = writeln!(out, "| `pg_doorman_servers_prepared_misses` | Live aggregate of prepared-statement cache misses across currently active backends of each pool, by user and database. This gauge can decrease when backends rotate; use `pg_doorman_servers_prepared_misses_total` for rates. |"); let _ = writeln!(out, "| `pg_doorman_servers_prepared_hits_total` | Counter form of prepared-statement cache hits across all backends of each pool, by user and database. Use `rate()` over this metric for hit throughput. |"); - let _ = writeln!(out, "| `pg_doorman_servers_prepared_misses_total` | Counter form of prepared-statement cache misses across all backends of each pool, by user and database. A sustained non-zero rate signals queries that could benefit from being prepared, or from a larger `server_prepared_statements_cache_size`. |\n"); + let _ = writeln!(out, "| `pg_doorman_servers_prepared_misses_total` | Counter form of prepared-statement cache misses across all backends of each pool, by user and database. A sustained non-zero rate signals queries that could benefit from being prepared, or from a larger `server_prepared_statements_cache_size`. |"); + let _ = writeln!(out, "| `pg_doorman_server_cleanup_total` | Counter by user, database and `result` (`ok` or `error`). Increments once per backend cleanup attempt that actually sent SQL to the backend: the built-in sequence in `adaptive`, `cleanup_server_query` in `always`, or a `ROLLBACK` of a transaction the client left open. In `off` mode only that `ROLLBACK` is counted, so a non-zero rate there means clients abandon transactions. `result=\"error\"` counts attempts that retired the backend: the cleanup query failed, timed out at `connect_timeout`, or left the backend not Idle. Checkins that needed no cleanup are not counted. The `database` label is the pool name. |\n"); // Per-Client Prepared Statement Cache Metrics let _ = writeln!(out, "### Per-Client Prepared Statement Cache Metrics\n"); diff --git a/src/app/generate/fields.yaml b/src/app/generate/fields.yaml index 1a24cd174..3e53da6ff 100644 --- a/src/app/generate/fields.yaml +++ b/src/app/generate/fields.yaml @@ -812,6 +812,33 @@ fields: `application_name` setting instead. default: "false" + cleanup_server_connections: + config: + en: | + Session cleanup mode for all pools: off, adaptive, or always. + Adaptive runs cleanup SQL only when the session state changed. + Always runs cleanup_server_query on every checkin. + Legacy false/true mean off/adaptive. + ru: | + Режим очистки сессии для всех пулов: off, adaptive или always. + Adaptive выполняет SQL очистки только при изменении состояния сессии. + Always выполняет cleanup_server_query при каждом возврате соединения. + Старые false/true означают off/adaptive. + doc: | + Session cleanup mode for every pool, unless the pool overrides it. `cleanup_server_query` is accepted by + `always` only. Legacy `false` and `true` mean `off` and `adaptive`. + See [pool settings](pool.md#cleanup_server_connections) for mode selection and limitations. + default: '"adaptive"' + + cleanup_server_query: + config: + en: "Session cleanup SQL for the always mode. Not allowed with adaptive." + ru: "SQL очистки сессии для режима always. С adaptive не допускается." + doc: | + Session cleanup SQL used by `always`. `adaptive` uses built-in cleanup and rejects a configured query. + See [pool settings](pool.md#cleanup_server_query) for examples and SQL requirements. + default: "None (built-in adaptive cleanup)" + message_size_to_be_stream: config: en: | @@ -1377,24 +1404,108 @@ fields: doc: "Override global sync_server_parameters for this pool. When enabled, client-sent session parameters (e.g. search_path, work_mem) are applied via SET queries on checkout. If not specified, the global setting is used." default: "None (uses global setting)" + cleanup_server_query: + config: + en: | + Override general.cleanup_server_query with custom session cleanup SQL. + Requires cleanup_server_connections = always; off does not use it. + ru: | + SQL очистки сессии вместо general.cleanup_server_query. + Требует cleanup_server_connections = always. В off не используется. + doc: | + SQL that replaces built-in cleanup. It runs on every checkin of a backend that served a client, and the + effective mode must be `always`: a pool in `adaptive` with its own or an inherited query is a configuration + error. In `off` the query is not used. A pool inherits `general.cleanup_server_query` unless it sets its own. + + The query must reset session parameters and release prepared statements, cursors and other application + resources. It must not leave a transaction open or enter COPY mode. An error or a `connect_timeout` timeout + closes the server connection. + + A good example is `cleanup_server_query = "DISCARD ALL"`, the query PgBouncer keeps as its default + `server_reset_query`. In PostgreSQL it is one command equivalent to `CLOSE ALL; + SET SESSION AUTHORIZATION DEFAULT; RESET ALL; DEALLOCATE ALL; UNLISTEN *; + SELECT pg_advisory_unlock_all(); DISCARD PLANS; DISCARD SEQUENCES; DISCARD TEMP`, so it is broader + than the built-in cleanup: it adds `SET SESSION AUTHORIZATION DEFAULT`, `DISCARD PLANS` and + `DISCARD SEQUENCES`. On Greengage `DISCARD ALL` has a quirk: it releases the temporary table + on the coordinator and leaves it on the segments. Example query for Greengage 7: + + ```sql + CLOSE ALL; + SET SESSION AUTHORIZATION DEFAULT; + RESET ALL; + DEALLOCATE ALL; + UNLISTEN *; + SELECT pg_advisory_unlock_all(); + DISCARD PLANS; + DISCARD SEQUENCES; + DISCARD TEMP + ``` + + `DISCARD TEMP` removes temporary tables from the coordinator and from the segments. Without it, temporary + objects and sequence state stay in the session. + + The cost for prepared statements. `DISCARD ALL` also removes prepared statements. In transaction + pooling, a statement is re-parsed by every transaction that uses it. In session pooling, statements + are reused inside one client connection. If the workload relies on prepared statements, use `adaptive`. + default: "None (uses global setting)" + cleanup_server_connections: config: en: | - Reset session state (SET, prepared statements, cursors) when returning a connection to pool. - ROLLBACK for open transactions is always executed regardless of this setting. - Prevents state leaking between clients in transaction mode. + Override general.cleanup_server_connections: off, adaptive, or always. + Adaptive runs cleanup SQL only when the session state changed. + Always runs cleanup_server_query on every checkin. Off does not disable ROLLBACK. ru: | - Сбрасывать состояние сессии (SET, prepared statements, курсоры) при возврате соединения в пул. - ROLLBACK для открытых транзакций выполняется всегда независимо от этой настройки. - Предотвращает утечку состояния между клиентами в транзакционном режиме. + Переопределить general.cleanup_server_connections: off, adaptive или always. + Adaptive выполняет SQL очистки только при изменении состояния сессии. + Always выполняет cleanup_server_query при каждом возврате соединения. Off не отключает ROLLBACK. doc: | - Controls whether pg_doorman resets session state when a connection is returned to the pool. - When enabled and the session was modified, pg_doorman sends: `RESET ROLE`, plus conditionally - `RESET ALL` (if SET was used), `DEALLOCATE ALL` (if PREPARE was used), `CLOSE ALL` (if cursors - were opened). Note: `ROLLBACK` for open transactions is always executed regardless of this setting. - Disable only if your application never uses SET, prepared statements, or cursors and you want - to save the cleanup roundtrip. - default: "true" + Session cleanup mode: + + - `adaptive` (default): built-in cleanup, triggered by tracked session state changes. Ordinary queries, + including cached prepared statements, add no cleanup SQL. A `cleanup_server_query` here is a configuration + error. + - `always`: `cleanup_server_query` runs on every checkin of a backend that served a client. The query is + required. + - `off`: no session cleanup. ROLLBACK of an unfinished transaction still runs. + + Built-in cleanup sends `RESET ROLE` and, when needed, `RESET ALL`, `DEALLOCATE ALL`, `CLOSE ALL`, + `UNLISTEN *` and `DISCARD TEMP`. A client that already sent `RESET ALL`, such as pgx, is not followed + by another `RESET ALL`. Every cleanup batch also releases advisory locks with + `pg_advisory_unlock_all()`. If the server lacks this function, the batch fails and the backend is + retired. + + Tracked statements and the matching cleanup: + + - `SET` → `RESET ALL`. + - `DECLARE` cursors → `CLOSE ALL`. + - SQL `PREPARE` and a prepared statement cache that no longer matches the server → `DEALLOCATE ALL`. + SQL `PREPARE` shares the server-side statement namespace with an extended-protocol `Parse`. + - `LISTEN` → `UNLISTEN *`. A client `UNLISTEN` does not suppress it: the command tag is shared by + `UNLISTEN ch` and `UNLISTEN *`, so a partial unsubscribe does not prove that no subscription remains. + - `CREATE TEMP TABLE` → `DISCARD TEMP`. The `CREATE TABLE` tag does not distinguish temp from permanent + tables, so a permanent `CREATE TABLE` arms `DISCARD TEMP` too; for permanent objects it is a no-op. + On Greengage `DISCARD TEMP` removes temporary tables from the coordinator and from the segments. + In transaction mode the backend returns to the pool after every query. A temporary table lives for one + transaction; keep a transaction open across the statements that use it, or use session mode. + + Not tracked by `adaptive`: + + - `SELECT ... INTO TEMP` and `CREATE TEMP TABLE AS SELECT`: current PostgreSQL completes them with the + inner query's tag, indistinguishable from a plain `SELECT`. On forks that still report the dedicated + tags, the cleanup arms. Temporary tables and sequences created inside functions carry no tag at all. + - Advisory locks: they carry no command tag, so a lock taken by a single statement does not arm cleanup. + When a batch runs for another reason, `pg_advisory_unlock_all()` releases them. + + Use `always` with SQL that releases the untracked resources. `RESET` of a single parameter and `RESET ALL` + return the same tag, so a client that resets one parameter suppresses the following `RESET ALL`, and other + changed parameters may stay set. In `always`, client `RESET` and `DISCARD ALL` do not replace the + configured query. + + Mode and query are inherited from `general` separately and validated together: `always` requires a query, + `adaptive` does not accept one. Legacy `true` and `false` mean `adaptive` and `off`. + A `RELOAD` applies a changed policy to new client connections. + default: "None (uses global setting)" prepared_statements_cache_size: config: diff --git a/src/app/generate/mod.rs b/src/app/generate/mod.rs index 37ad143a6..d671262e5 100644 --- a/src/app/generate/mod.rs +++ b/src/app/generate/mod.rs @@ -139,7 +139,8 @@ pub fn generate_config_with_client( connect_timeout: None, idle_timeout: None, server_lifetime: None, - cleanup_server_connections: false, + cleanup_server_connections: Some(crate::config::CleanupMode::Off), + cleanup_server_query: None, log_client_parameter_status_changes: false, application_name: None, server_host: config diff --git a/src/app/generate/tests.rs b/src/app/generate/tests.rs index 0a9bdfc86..bece7b637 100644 --- a/src/app/generate/tests.rs +++ b/src/app/generate/tests.rs @@ -62,7 +62,8 @@ pub fn generate_config_with_client< connect_timeout: None, idle_timeout: None, server_lifetime: None, - cleanup_server_connections: false, + cleanup_server_connections: Some(crate::config::CleanupMode::Off), + cleanup_server_query: None, log_client_parameter_status_changes: false, application_name: None, server_host: config diff --git a/src/client/transaction.rs b/src/client/transaction.rs index 19611f0b7..d5eeb8c2f 100644 --- a/src/client/transaction.rs +++ b/src/client/transaction.rs @@ -448,6 +448,11 @@ where } } + let cleanup_result = if conn.custom_cleanup_enabled() { + conn.cleanup_after_response().await + } else { + Ok(()) + }; write_all_flush(&mut self.write, &response).await?; if !has_error_response(&response) && ends_with_idle_ready_for_query(&response) { @@ -455,7 +460,7 @@ where .set(snapshot.query.clone(), response.freeze()); } - Ok(()) + cleanup_result } /// Handle simple query (Q message). @@ -880,13 +885,11 @@ where // checkin_cleanup before give server to client. match conn.checkin_cleanup().await { Ok(()) => break conn, - Err(err) => { - warn!( - "[{}@{} #c{}] server cleanup error: {err}", - self.username, self.pool_name, self.connection_id, - ); - continue; - } + // The error is already logged by checkin_cleanup, which also + // marks the backend bad. Dropping `conn` closes it, so the + // checkout retries with the next candidate instead of + // reporting a pool failure to the client. + Err(_) => continue, }; } Err(err) => { @@ -1141,7 +1144,7 @@ where Err(err) => { self.stats.disconnect(); self.connected_to_server = false; - server.checkin_cleanup().await?; + let _ = server.checkin_cleanup().await; self.release(); return self.process_error(err).await; } @@ -1184,7 +1187,7 @@ where // Terminate 'X' => { - server.checkin_cleanup().await?; + let _ = server.checkin_cleanup().await; self.stats.disconnect(); self.connected_to_server = false; self.release(); @@ -1266,7 +1269,7 @@ where if shutdown_in_progress { server.mark_bad("graceful shutdown - releasing server connection"); } else if !server.is_async() { - server.checkin_cleanup().await?; + server.cleanup_after_response().await?; } if self.transaction_mode { server diff --git a/src/config/general.rs b/src/config/general.rs index 3664435be..8f23cb101 100644 --- a/src/config/general.rs +++ b/src/config/general.rs @@ -4,7 +4,7 @@ use ipnet::IpNet; use serde_derive::{Deserialize, Serialize}; use super::tls; -use super::{ByteSize, Duration, Include}; +use super::{ByteSize, CleanupMode, Duration, Include}; use crate::auth::hba::PgHba; /// General configuration. @@ -141,6 +141,13 @@ pub struct General { #[serde(default = "General::default_sync_server_parameters")] // False pub sync_server_parameters: bool, + #[serde(default)] + pub cleanup_server_connections: CleanupMode, + + /// Default cleanup query; pools may override it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cleanup_server_query: Option, + #[serde(default = "General::default_worker_threads")] pub worker_threads: usize, @@ -570,6 +577,8 @@ impl Default for General { log_client_connections: true, log_client_disconnections: true, sync_server_parameters: Self::default_sync_server_parameters(), + cleanup_server_connections: CleanupMode::Adaptive, + cleanup_server_query: None, tls_certificate: None, tls_private_key: None, tls_ca_cert: None, diff --git a/src/config/mod.rs b/src/config/mod.rs index 26e1eda00..c76b4a7c2 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -54,6 +54,49 @@ pub use web::Web; pub const VERSION: &str = env!("CARGO_PKG_VERSION"); +/// Backend cleanup policy. Legacy booleans map to adaptive/off. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum CleanupMode { + Off, + #[default] + Adaptive, + Always, +} + +impl<'de> serde::Deserialize<'de> for CleanupMode { + fn deserialize>(deserializer: D) -> Result { + struct CleanupModeVisitor; + + impl<'de> serde::de::Visitor<'de> for CleanupModeVisitor { + type Value = CleanupMode; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a boolean or one of: off, adaptive, always") + } + + fn visit_bool(self, value: bool) -> Result { + Ok(if value { + CleanupMode::Adaptive + } else { + CleanupMode::Off + }) + } + + fn visit_str(self, value: &str) -> Result { + match value { + "off" => Ok(CleanupMode::Off), + "adaptive" => Ok(CleanupMode::Adaptive), + "always" => Ok(CleanupMode::Always), + _ => Err(E::invalid_value(serde::de::Unexpected::Str(value), &self)), + } + } + } + + deserializer.deserialize_any(CleanupModeVisitor) + } +} + /// Configuration file format. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ConfigFormat { @@ -237,6 +280,19 @@ pub struct Config { pub include: Include, } +fn validate_cleanup_server_query(query: &str) -> Result<(), Error> { + if query + .trim_matches(|c: char| c.is_whitespace() || c == ';') + .is_empty() + || query.contains('\0') + { + return Err(Error::BadConfig( + "cleanup_server_query must contain SQL and no NUL bytes".into(), + )); + } + Ok(()) +} + impl Config { pub fn default_path() -> String { String::from("pg_doorman.toml") @@ -381,8 +437,9 @@ impl Config { pool_name, pool.server_host, pool.server_port ); info!( - "[pool: {}] Cleanup server connections: {}", - pool_name, pool.cleanup_server_connections + "[pool: {}] Cleanup server connections: {:?}", + pool_name, + pool.effective_cleanup_server_connections(&self.general) ); info!( "[pool: {}] Connect timeout: {}", @@ -423,6 +480,9 @@ impl Config { /// Validate the configuration. pub async fn validate(&mut self) -> Result<(), Error> { + if let Some(query) = &self.general.cleanup_server_query { + validate_cleanup_server_query(query)?; + } // Validate Talos self.talos.validate().await?; @@ -762,8 +822,21 @@ impl Config { } } - for pool in self.pools.values_mut() { + for (name, pool) in &mut self.pools { pool.validate().await?; + let mode = pool.effective_cleanup_server_connections(&self.general); + let query = pool.effective_cleanup_server_query(&self.general); + if mode == CleanupMode::Always && query.is_none() { + return Err(Error::BadConfig(format!( + "pools.{name}.cleanup_server_connections = always requires cleanup_server_query" + ))); + } + if mode == CleanupMode::Adaptive && query.is_some() { + return Err(Error::BadConfig(format!( + "pools.{name}.cleanup_server_query requires cleanup_server_connections = always; \ + adaptive uses built-in cleanup only" + ))); + } } // Cross-config validation: coordinator timeouts vs query_wait_timeout diff --git a/src/config/pool.rs b/src/config/pool.rs index 5dd3944e3..5f9664a2e 100644 --- a/src/config/pool.rs +++ b/src/config/pool.rs @@ -9,7 +9,7 @@ use std::collections::HashSet; use std::fmt; use std::hash::{Hash, Hasher}; -use super::{Duration, PoolMode, User}; +use super::{CleanupMode, Duration, PoolMode, User}; /// Custom deserializer for users field that supports both formats: /// - Array format (recommended): `users: [{ username: "user1", ... }]` @@ -79,8 +79,12 @@ pub struct Pool { #[serde(skip_serializing_if = "Option::is_none")] pub server_lifetime: Option, - #[serde(default = "Pool::default_cleanup_server_connections")] - pub cleanup_server_connections: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cleanup_server_connections: Option, + + /// Full session reset; unset inherits general.cleanup_server_query. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cleanup_server_query: Option, #[serde(default)] // False pub log_client_parameter_status_changes: bool, @@ -227,10 +231,6 @@ impl Pool { String::from("127.0.0.1") } - pub fn default_cleanup_server_connections() -> bool { - true - } - /// Resolve scaling config by merging pool-level overrides with general defaults. /// Anticipation/burst params are global-only by design (no per-pool override). pub fn resolve_scaling_config( @@ -250,7 +250,30 @@ impl Pool { } } + pub fn effective_cleanup_server_connections(&self, general: &super::General) -> CleanupMode { + self.cleanup_server_connections + .unwrap_or(general.cleanup_server_connections) + } + + pub fn effective_cleanup_server_query<'a>( + &'a self, + general: &'a super::General, + ) -> Option<&'a str> { + self.cleanup_server_query + .as_deref() + .or(general.cleanup_server_query.as_deref()) + } + + pub fn effective_connect_timeout(&self, general: &super::General) -> std::time::Duration { + self.connect_timeout + .map(std::time::Duration::from_millis) + .unwrap_or(general.connect_timeout.as_std()) + } + pub async fn validate(&mut self) -> Result<(), Error> { + if let Some(query) = &self.cleanup_server_query { + super::validate_cleanup_server_query(query)?; + } crate::config::startup_parameters::validate( &self.startup_parameters, "pool.startup_parameters", @@ -459,7 +482,8 @@ impl Default for Pool { connect_timeout: None, idle_timeout: None, server_lifetime: None, - cleanup_server_connections: true, + cleanup_server_connections: None, + cleanup_server_query: None, log_client_parameter_status_changes: false, application_name: None, prepared_statements_cache_size: None, diff --git a/src/config/tests.rs b/src/config/tests.rs index 483fb887a..b00fc7887 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -2202,3 +2202,116 @@ pool_size = 10 let pool = &config.pools["mydb"]; assert_eq!(pool.sync_server_parameters, None); } + +#[test] +fn cleanup_modes_parse_legacy_bools_and_serialize_strings() { + for (input, expected) in [ + ("true", CleanupMode::Adaptive), + ("false", CleanupMode::Off), + ("\"off\"", CleanupMode::Off), + ("\"adaptive\"", CleanupMode::Adaptive), + ("\"always\"", CleanupMode::Always), + ] { + for (format, content) in [ + (ConfigFormat::Toml, format!("[general]\nadmin_username = \"admin\"\nadmin_password = \"admin\"\ncleanup_server_connections = {input}\n[pools.test]\ncleanup_server_connections = {input}")), + (ConfigFormat::Yaml, format!("general:\n admin_username: admin\n admin_password: admin\n cleanup_server_connections: {input}\npools:\n test:\n cleanup_server_connections: {input}")), + ] { + let config: Config = parse_config_content(&content, format).unwrap(); + assert_eq!(config.general.cleanup_server_connections, expected); + assert_eq!(config.pools["test"].cleanup_server_connections, Some(expected)); + assert!(serde_json::to_value(expected).unwrap().is_string()); + } + } + for invalid in ["1", "\"true\"", "\"Adaptive\"", "\"auto\"", "\"\""] { + assert!( + toml::from_str::(&format!("admin_username = \"admin\"\nadmin_password = \"admin\"\ncleanup_server_connections = {invalid}")).is_err() + ); + assert!(serde_yaml::from_str::(&format!( + "admin_username: admin\nadmin_password: admin\ncleanup_server_connections: {invalid}" + )) + .is_err()); + } + assert_eq!( + serde_json::to_string(&CleanupMode::Adaptive).unwrap(), + "\"adaptive\"" + ); +} + +#[tokio::test] +async fn cleanup_policy_inheritance_validation_and_pool_hash() { + let mut config = Config::default(); + config.pools.insert("test".into(), Pool::default()); + assert_eq!(config.pools["test"].cleanup_server_connections, None); + assert_eq!( + config.pools["test"].effective_cleanup_server_connections(&config.general), + CleanupMode::Adaptive + ); + let default_hash = config.pools["test"].hash_value(); + config.general.cleanup_server_connections = CleanupMode::Always; + assert!( + matches!(config.validate().await, Err(Error::BadConfig(message)) + if message.contains("pools.test.cleanup_server_connections") && message.contains("cleanup_server_query")) + ); + config.pools.get_mut("test").unwrap().cleanup_server_query = Some("DISCARD ALL".into()); + config.validate().await.unwrap(); // A pool query satisfies inherited Always. + assert_ne!(config.pools["test"].hash_value(), default_hash); + config.general.cleanup_server_query = Some("RESET ALL".into()); + assert_eq!( + config.pools["test"].effective_cleanup_server_query(&config.general), + Some("DISCARD ALL") + ); + config.pools.get_mut("test").unwrap().cleanup_server_query = None; + assert_eq!( + config.pools["test"].effective_cleanup_server_query(&config.general), + Some("RESET ALL") + ); + config + .pools + .get_mut("test") + .unwrap() + .cleanup_server_connections = Some(CleanupMode::Adaptive); + assert_eq!( + config.pools["test"].effective_cleanup_server_connections(&config.general), + CleanupMode::Adaptive + ); + config.general.cleanup_server_query = None; + config.validate().await.unwrap(); // Explicit adaptive overrides general Always without a query. + config + .pools + .get_mut("test") + .unwrap() + .cleanup_server_connections = Some(CleanupMode::Off); + config.general.cleanup_server_query = Some("DISCARD ALL".into()); + config.validate().await.unwrap(); // Off ignores a valid inherited query. + for query in ["", " \t\n", "; ;\n;", "RESET ALL;\0DEALLOCATE ALL"] { + config.general.cleanup_server_query = Some(query.into()); + assert!( + matches!(config.validate().await, Err(Error::BadConfig(message)) if message.contains("cleanup_server_query")) + ); + config.general.cleanup_server_query = None; + config.pools.get_mut("test").unwrap().cleanup_server_query = Some(query.into()); + assert!( + matches!(config.validate().await, Err(Error::BadConfig(message)) if message.contains("cleanup_server_query")) + ); + } + + // An adaptive pool rejects its own or an inherited query. + config.general.cleanup_server_connections = CleanupMode::Adaptive; + config.general.cleanup_server_query = Some("DISCARD ALL".into()); + config + .pools + .get_mut("test") + .unwrap() + .cleanup_server_connections = Some(CleanupMode::Adaptive); + config.pools.get_mut("test").unwrap().cleanup_server_query = None; + assert!( + matches!(config.validate().await, Err(Error::BadConfig(message)) + if message.contains("pools.test.cleanup_server_query") && message.contains("requires cleanup_server_connections = always")) + ); + config.general.cleanup_server_query = None; + config.pools.get_mut("test").unwrap().cleanup_server_query = Some("DISCARD ALL".into()); + assert!( + matches!(config.validate().await, Err(Error::BadConfig(message)) + if message.contains("pools.test.cleanup_server_query") && message.contains("requires cleanup_server_connections = always")) + ); +} diff --git a/src/pool/dynamic.rs b/src/pool/dynamic.rs index d4100a2fb..85873a649 100644 --- a/src/pool/dynamic.rs +++ b/src/pool/dynamic.rs @@ -191,7 +191,10 @@ pub fn create_dynamic_pool( user.clone(), server_database.as_str(), client_server_map, - pool_config.cleanup_server_connections, + pool_config.effective_cleanup_server_connections(&config.general), + pool_config + .effective_cleanup_server_query(&config.general) + .map(str::to_owned), pool_config.log_client_parameter_status_changes, server_prepared_statements_cache_size, application_name, diff --git a/src/pool/gc.rs b/src/pool/gc.rs index b3376a612..c439e26ab 100644 --- a/src/pool/gc.rs +++ b/src/pool/gc.rs @@ -134,7 +134,8 @@ mod tests { }, "test_db", Arc::new(DashMap::new()), - false, + crate::config::CleanupMode::Off, + None, false, 0, "test_app".to_string(), diff --git a/src/pool/inner.rs b/src/pool/inner.rs index 6618bac40..6cf029b9c 100644 --- a/src/pool/inner.rs +++ b/src/pool/inner.rs @@ -2088,7 +2088,8 @@ mod tests { User::default(), "test_db", Arc::new(DashMap::new()), - false, + crate::config::CleanupMode::Off, + None, false, 0, "test_app".to_string(), @@ -2239,7 +2240,8 @@ mod tests { User::default(), "test_db", Arc::new(DashMap::new()), - false, + crate::config::CleanupMode::Off, + None, false, 0, "test_app".to_string(), diff --git a/src/pool/mod.rs b/src/pool/mod.rs index ca9c611c7..5adc2089d 100644 --- a/src/pool/mod.rs +++ b/src/pool/mod.rs @@ -406,7 +406,7 @@ impl ConnectionPool { } // Hashing each pool's effective config against (Pool, general - // startup_parameters baseline + sync_server_parameters) folds + // startup_parameters baseline + sync_server_parameters + reset query) folds // general-level GUC changes into the same reuse decision pg_doorman // already uses for pool-level changes. Without this, a SIGHUP that // only edits `general.startup_parameters` or @@ -419,6 +419,8 @@ impl ConnectionPool { let mut hasher = std::collections::hash_map::DefaultHasher::new(); config.general.startup_parameters.hash(&mut hasher); config.general.sync_server_parameters.hash(&mut hasher); + config.general.cleanup_server_query.hash(&mut hasher); + config.general.cleanup_server_connections.hash(&mut hasher); hasher.finish() }; // Load only; the hash is not advanced until the new pool map has @@ -564,7 +566,10 @@ impl ConnectionPool { user.clone(), server_database.as_str(), client_server_map.clone(), - pool_config.cleanup_server_connections, + pool_config.effective_cleanup_server_connections(&config.general), + pool_config + .effective_cleanup_server_query(&config.general) + .map(str::to_owned), pool_config.log_client_parameter_status_changes, server_prepared_statements_cache_size, application_name, @@ -629,7 +634,8 @@ impl ConnectionPool { life_time_ms: pool_config .server_lifetime .unwrap_or(config.general.server_lifetime.as_millis()), - sync_server_parameters: pool_config.effective_sync_server_parameters(&config.general), + sync_server_parameters: pool_config + .effective_sync_server_parameters(&config.general), min_guaranteed_pool_size: pool_config.min_guaranteed_pool_size.unwrap_or(0), }, prepared_statement_cache: match config.general.prepared_statements { @@ -781,7 +787,10 @@ impl ConnectionPool { shared_user.clone(), server_database.as_str(), client_server_map.clone(), - pool_config.cleanup_server_connections, + pool_config.effective_cleanup_server_connections(&config.general), + pool_config + .effective_cleanup_server_query(&config.general) + .map(str::to_owned), pool_config.log_client_parameter_status_changes, server_prepared_statements_cache_size, application_name, @@ -858,7 +867,8 @@ impl ConnectionPool { life_time_ms: pool_config .server_lifetime .unwrap_or(config.general.server_lifetime.as_millis()), - sync_server_parameters: pool_config.effective_sync_server_parameters(&config.general), + sync_server_parameters: pool_config + .effective_sync_server_parameters(&config.general), min_guaranteed_pool_size: pool_config .min_guaranteed_pool_size .unwrap_or(0), @@ -1520,6 +1530,8 @@ mod tests { let mut hasher = std::collections::hash_map::DefaultHasher::new(); general.startup_parameters.hash(&mut hasher); general.sync_server_parameters.hash(&mut hasher); + general.cleanup_server_query.hash(&mut hasher); + general.cleanup_server_connections.hash(&mut hasher); hasher.finish() } @@ -1606,6 +1618,31 @@ mod tests { ); } + #[test] + fn reload_general_cleanup_policy_changes_fingerprints() { + let pool = ConfigPool::default(); + let before = General::default(); + for after in [ + General { + cleanup_server_query: Some("DISCARD ALL".into()), + ..General::default() + }, + General { + cleanup_server_connections: crate::config::CleanupMode::Off, + ..General::default() + }, + ] { + assert_ne!( + compute_pool_fingerprint(&pool, &before), + compute_pool_fingerprint(&pool, &after) + ); + assert_ne!( + pool.hash_value() ^ compute_general_startup_hash(&before), + pool.hash_value() ^ compute_general_startup_hash(&after) + ); + } + } + #[test] fn reload_no_change_preserves_fingerprint() { let general = General::default(); diff --git a/src/pool/retain.rs b/src/pool/retain.rs index 73ee38429..dceaed6af 100644 --- a/src/pool/retain.rs +++ b/src/pool/retain.rs @@ -310,7 +310,8 @@ mod tests { User::default(), "test_db", Arc::new(DashMap::new()), - false, + crate::config::CleanupMode::Off, + None, false, 0, "test_app".to_string(), diff --git a/src/pool/server_pool.rs b/src/pool/server_pool.rs index ae63bc2b0..429386c4e 100644 --- a/src/pool/server_pool.rs +++ b/src/pool/server_pool.rs @@ -13,7 +13,7 @@ use log::{debug, info, warn}; use tokio::sync::{Notify, Semaphore}; use crate::config::startup_parameters as sp; -use crate::config::{Address, User}; +use crate::config::{Address, CleanupMode, User}; use crate::errors::Error; use crate::patroni::types::Role; use crate::server::Server; @@ -175,7 +175,8 @@ pub struct ServerPool { client_server_map: ClientServerMap, /// Should we clean up dirty connections before putting them into the pool? - cleanup_connections: bool, + cleanup_connections: CleanupMode, + cleanup_server_query: Option, application_name: String, @@ -289,7 +290,8 @@ impl ServerPool { user: User, database: &str, client_server_map: ClientServerMap, - cleanup_connections: bool, + cleanup_connections: CleanupMode, + cleanup_server_query: Option, log_client_parameter_status_changes: bool, prepared_statement_cache_size: usize, application_name: String, @@ -330,6 +332,7 @@ impl ServerPool { database: database.to_string(), client_server_map, cleanup_connections, + cleanup_server_query, log_client_parameter_status_changes, prepared_statement_cache_size, create_semaphore: Arc::new(Semaphore::new(max_concurrent_creates)), @@ -434,6 +437,7 @@ impl ServerPool { self.client_server_map.clone(), stats.clone(), self.cleanup_connections, + self.cleanup_server_query.clone(), self.log_client_parameter_status_changes, self.prepared_statement_cache_size, self.application_name.clone(), @@ -462,10 +466,17 @@ impl ServerPool { _ => false, }; let (result, active_registration) = if should_tls_retry { + // Read before the retry shadows `result`: the plain attempt itself + // logs nothing, so this is the only place its failure survives. + let plain_error = result + .as_ref() + .err() + .map(ToString::to_string) + .unwrap_or_default(); info!( - "plain connection rejected, retrying with tls, user={} pool={} host={} port={} server_tls_mode=allow", + "plain connection rejected, retrying with tls, user={} pool={} host={} port={} server_tls_mode=allow err={}", self.address.username, self.address.pool_name, - self.address.host, self.address.port, + self.address.host, self.address.port, plain_error, ); // The plain attempt ended; keep only the TLS retry row visible. drop(registration); @@ -489,6 +500,7 @@ impl ServerPool { self.client_server_map.clone(), retry_stats.clone(), self.cleanup_connections, + self.cleanup_server_query.clone(), self.log_client_parameter_status_changes, self.prepared_statement_cache_size, self.application_name.clone(), @@ -1062,6 +1074,7 @@ impl ServerPool { self.client_server_map.clone(), stats.clone(), self.cleanup_connections, + self.cleanup_server_query.clone(), self.log_client_parameter_status_changes, self.prepared_statement_cache_size, self.application_name.clone(), @@ -1085,12 +1098,18 @@ impl ServerPool { _ => false, }; let (result, active_registration) = if should_tls_retry { + let plain_error = result + .as_ref() + .err() + .map(ToString::to_string) + .unwrap_or_default(); info!( - "[{}@{}] fallback: plain connection to {}:{} rejected, retrying with tls", + "[{}@{}] fallback: plain connection to {}:{} rejected, retrying with tls, err={}", self.address.username, self.address.pool_name, fallback_address.host, fallback_address.port, + plain_error, ); // The plain candidate ended; keep only the TLS retry row visible. drop(registration); @@ -1114,6 +1133,7 @@ impl ServerPool { self.client_server_map.clone(), retry_stats.clone(), self.cleanup_connections, + self.cleanup_server_query.clone(), self.log_client_parameter_status_changes, self.prepared_statement_cache_size, self.application_name.clone(), diff --git a/src/server/cleanup.rs b/src/server/cleanup.rs index 716c2f507..3bfbb71b3 100644 --- a/src/server/cleanup.rs +++ b/src/server/cleanup.rs @@ -8,6 +8,18 @@ pub(crate) struct CleanupState { /// If server connection requires CLOSE ALL before checkin because of declare statement pub(crate) needs_cleanup_declare: bool, + + /// If server connection requires UNLISTEN * before checkin because of a LISTEN + /// subscription. Client UNLISTEN does not disarm: the tag is shared by + /// `UNLISTEN ch` and `UNLISTEN *`, so a partial unsubscribe cannot prove + /// that no subscription remains. + pub(crate) needs_cleanup_listen: bool, + + /// If server connection requires DISCARD TEMP before checkin because a + /// temp-creating tag (CREATE TABLE, CREATE TABLE AS, SELECT INTO) was seen. + /// The tags do not distinguish temp from permanent objects, so a permanent + /// CREATE TABLE arms the flag too; DISCARD TEMP is a no-op then. + pub(crate) needs_cleanup_temp: bool, } impl CleanupState { @@ -16,12 +28,18 @@ impl CleanupState { needs_cleanup_set: false, needs_cleanup_prepare: false, needs_cleanup_declare: false, + needs_cleanup_listen: false, + needs_cleanup_temp: false, } } #[inline(always)] pub(crate) fn needs_cleanup(&self) -> bool { - self.needs_cleanup_set || self.needs_cleanup_prepare || self.needs_cleanup_declare + self.needs_cleanup_set + || self.needs_cleanup_prepare + || self.needs_cleanup_declare + || self.needs_cleanup_listen + || self.needs_cleanup_temp } #[inline(always)] @@ -29,6 +47,8 @@ impl CleanupState { self.needs_cleanup_set = true; self.needs_cleanup_prepare = true; self.needs_cleanup_declare = true; + self.needs_cleanup_listen = true; + self.needs_cleanup_temp = true; } #[inline(always)] @@ -36,6 +56,8 @@ impl CleanupState { self.needs_cleanup_set = false; self.needs_cleanup_prepare = false; self.needs_cleanup_declare = false; + self.needs_cleanup_listen = false; + self.needs_cleanup_temp = false; } } @@ -43,8 +65,12 @@ impl std::fmt::Display for CleanupState { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "SET: {}, PREPARE: {}, DECLARE: {}", - self.needs_cleanup_set, self.needs_cleanup_prepare, self.needs_cleanup_declare + "SET: {}, PREPARE: {}, DECLARE: {}, LISTEN: {}, TEMP: {}", + self.needs_cleanup_set, + self.needs_cleanup_prepare, + self.needs_cleanup_declare, + self.needs_cleanup_listen, + self.needs_cleanup_temp ) } } diff --git a/src/server/parameters.rs b/src/server/parameters.rs index fd6a5254e..54e524e6c 100644 --- a/src/server/parameters.rs +++ b/src/server/parameters.rs @@ -38,6 +38,9 @@ static SET_FORBIDDEN_PARAMETERS: Lazy> = Lazy::new(|| { s.insert("max_function_args"); s.insert("data_checksums"); s.insert("data_directory_mode"); + // Greengage reports these at startup, but they cannot change per session. + s.insert("gp_server_version"); + s.insert("gp_autovacuum_scope"); // Database-level GUCs cannot vary per session. s.insert("lc_collate"); s.insert("lc_ctype"); @@ -240,6 +243,14 @@ impl ServerParameters { } } + /// A full reset changes GUCs that PostgreSQL does not report in ParameterStatus. + pub(crate) fn forget_untracked(&mut self) { + self.parameters + .retain(|key, _| TRACKED_PARAMETERS.contains(key) || is_set_forbidden(key)); + self.planner_hash_cache + .store(PLANNER_HASH_UNSET, std::sync::atomic::Ordering::Relaxed); + } + /// Diff the backend snapshot (`self`) against the client's desired /// state and return the SET/RESET actions needed for checkout sync. /// Forbidden names are skipped on both passes. @@ -552,6 +563,27 @@ mod tests { assert!(!diff.contains_key("user")); } + #[test] + fn cleanup_sync_restores_startup_state_without_replaying_greengage_metadata() { + let mut backend = ServerParameters::new(); + backend.set_param("gp_autovacuum_scope", "catalog", true); + backend.set_param("gp_server_version", "7.5.0", true); + backend.set_param("search_path", "startup_schema", true); + let client = backend.clone(); + + backend.forget_untracked(); + + assert_eq!( + backend.compare_params(&client), + HashMap::from([( + "search_path".to_string(), + ParamAction::SetTo("startup_schema".to_string()), + )]) + ); + // A client without these server-reported values must not RESET them. + assert!(backend.compare_params(&ServerParameters::new()).is_empty()); + } + #[test] fn planner_param_hash_distinguishes_non_search_path_planner_key() { // `role` also changes planner state and must affect the digest. diff --git a/src/server/protocol_io.rs b/src/server/protocol_io.rs index b1cf66b72..7ad97defe 100644 --- a/src/server/protocol_io.rs +++ b/src/server/protocol_io.rs @@ -36,23 +36,12 @@ use super::parameters::ServerParameters; use super::server_backend::Server; // PostgreSQL CommandComplete message payloads for tracking session state changes. -// -// A checkin-time `RESET ALL` / `DEALLOCATE ALL` / `CLOSE ALL` is a heuristic -// upper bound: we arm the `needs_cleanup_*` flags when we see a statement that -// *might* have mutated the session, and we disarm them when we see a statement -// that has since restored it. Disarming matters because otherwise a client that -// performs its own reset batch (e.g. pgx on internal context deadline sends -// `SET SESSION AUTHORIZATION DEFAULT; RESET ALL; CLOSE ALL; UNLISTEN *; -// DISCARD PLANS; ...`) leaves pg_doorman thinking the connection is still dirty -// and triggers a second, redundant `RESET ALL` round-trip on checkin. /// `SET` statement CommandComplete tag — arms the `needs_cleanup_set` flag. /// Returned for any `SET foo = ...`, including `SET SESSION AUTHORIZATION ...`. const COMMAND_COMPLETE_BY_SET: &[u8; 4] = b"SET\0"; -/// `RESET` statement CommandComplete tag — disarms `needs_cleanup_set`. -/// PostgreSQL returns this tag both for `RESET ALL` and for `RESET foo.bar`; -/// the per-GUC form is still safe to treat as a reset because the only state -/// pg_doorman tracked is the `SET` flag — the next `SET` will re-arm it. +/// `RESET` has the same tag for ALL and a single GUC. Either suppresses built-in +/// SET cleanup; custom cleanup remains pending. const COMMAND_COMPLETE_BY_RESET: &[u8; 6] = b"RESET\0"; /// `DECLARE CURSOR` CommandComplete tag — arms the `needs_cleanup_declare` flag. const COMMAND_COMPLETE_BY_DECLARE: &[u8; 15] = b"DECLARE CURSOR\0"; @@ -65,6 +54,25 @@ const COMMAND_COMPLETE_BY_DEALLOCATE_ALL: &[u8; 15] = b"DEALLOCATE ALL\0"; /// `DISCARD ALL` CommandComplete tag — equivalent to `RESET ALL; DEALLOCATE ALL; /// CLOSE ALL; UNLISTEN *; ...`, so disarms every `needs_cleanup_*` flag. const COMMAND_COMPLETE_BY_DISCARD_ALL: &[u8; 12] = b"DISCARD ALL\0"; +/// `LISTEN` CommandComplete tag — arms the `needs_cleanup_listen` flag. +/// `UNLISTEN` shares one tag for `UNLISTEN ch` and `UNLISTEN *`, so it cannot +/// disarm: a partial unsubscribe leaves the remaining subscriptions behind. +const COMMAND_COMPLETE_BY_LISTEN: &[u8; 7] = b"LISTEN\0"; +/// SQL `PREPARE` CommandComplete tag — arms `needs_cleanup_prepare`. The SQL +/// command shares the server-side statement namespace with an extended-protocol +/// `Parse`, which pg_doorman tracks separately through its cache bookkeeping. +const COMMAND_COMPLETE_BY_PREPARE: &[u8; 8] = b"PREPARE\0"; +/// `CREATE TABLE` CommandComplete tag — arms the `needs_cleanup_temp` flag. +/// The tag does not distinguish `CREATE TEMP TABLE` from a permanent +/// `CREATE TABLE`; the false positive only costs one `DISCARD TEMP` in the +/// already-sent cleanup batch. +const COMMAND_COMPLETE_BY_CREATE_TABLE: &[u8; 13] = b"CREATE TABLE\0"; +/// Legacy `CREATE TABLE AS` / `SELECT INTO` tags — arm the `needs_cleanup_temp` +/// flag for forks that still emit them. Current PostgreSQL answers these +/// statements with the inner query's tag (`SELECT `), which is +/// indistinguishable from a plain `SELECT` and stays untracked. +const COMMAND_COMPLETE_BY_CREATE_TABLE_AS: &[u8; 16] = b"CREATE TABLE AS\0"; +const COMMAND_COMPLETE_BY_SELECT_INTO: &[u8; 12] = b"SELECT INTO\0"; /// Buffer flush threshold in bytes (8 KiB). /// When the buffer reaches this size, it will be flushed to avoid excessive memory usage. @@ -438,8 +446,16 @@ enum CommandCompleteEffect { ArmSet, /// `DECLARE CURSOR` — a server-side cursor may now be open; arm declare-cleanup. ArmDeclare, - /// `RESET` / `RESET ALL` — session GUCs are back to the server defaults; - /// disarm set-cleanup because the subsequent checkin RESET would be a no-op. + /// `LISTEN` — a session subscription may now be registered; arm listen-cleanup. + ArmListen, + /// SQL `PREPARE` — a named statement now exists server-side; arm prepare-cleanup. + ArmPrepare, + /// `CREATE TABLE` (and the legacy `CREATE TABLE AS` / `SELECT INTO` tags of + /// older forks) — a temp table may now exist; arm temp-cleanup. The tags do + /// not distinguish temp from permanent objects; `DISCARD TEMP` is a no-op + /// for permanent ones. + ArmTemp, + /// `RESET` / `RESET ALL` — disarm built-in SET cleanup. DisarmSet, /// `CLOSE CURSOR ALL` — no server-side cursors remain; disarm declare-cleanup. DisarmDeclare, @@ -460,6 +476,15 @@ enum CommandCompleteEffect { fn classify_command_complete(tag: &[u8]) -> CommandCompleteEffect { if tag == COMMAND_COMPLETE_BY_SET { CommandCompleteEffect::ArmSet + } else if tag == COMMAND_COMPLETE_BY_LISTEN { + CommandCompleteEffect::ArmListen + } else if tag == COMMAND_COMPLETE_BY_PREPARE { + CommandCompleteEffect::ArmPrepare + } else if tag == COMMAND_COMPLETE_BY_CREATE_TABLE + || tag == COMMAND_COMPLETE_BY_CREATE_TABLE_AS + || tag == COMMAND_COMPLETE_BY_SELECT_INTO + { + CommandCompleteEffect::ArmTemp } else if tag == COMMAND_COMPLETE_BY_RESET { CommandCompleteEffect::DisarmSet } else if tag == COMMAND_COMPLETE_BY_DECLARE { @@ -486,23 +511,25 @@ fn drop_prepared_statement_cache_on_reset(server: &mut Server, reason: &'static else { return; }; - warn!( - "[{}@{}] clearing prepared statement cache pid={}: {reason} ({cache_size} entries)", - server.address.username, - server.address.pool_name, - server.get_process_id(), - ); + if !server.resetting && cache_size > 0 { + warn!( + "[{}@{}] clearing prepared statement cache pid={}: {reason} ({cache_size} entries)", + server.address.username, + server.address.pool_name, + server.get_process_id(), + ); + } if let Some(cache) = server.prepared_statement_cache.as_mut() { cache.clear(); } } /// Handles CommandComplete ('C') message - indicates successful completion of a command. -/// Tracks commands that may require cleanup (SET, DECLARE, ...) and disarms the -/// cleanup flags when the session has since been restored by a RESET / DISCARD / -/// DEALLOCATE / CLOSE ALL statement in the same or a later batch — so that the -/// next checkin does not issue a redundant `RESET ALL` round-trip on a connection -/// the client has already cleaned up. +/// Tracks commands that may require cleanup (SET, DECLARE, LISTEN, SQL PREPARE, +/// temp-creating DDL) and disarms the cleanup flags after DISCARD / DEALLOCATE / +/// CLOSE ALL. Client RESET and DISCARD ALL suppress built-in cleanup; the `always` +/// mode runs its configured query regardless. Client `UNLISTEN` and single-statement +/// `DEALLOCATE`/`DROP` share their tag with the unconditional forms and never disarm. fn handle_command_complete(server: &mut Server, message: &BytesMut) { // Exit COPY mode if we were in it if server.in_copy_mode { @@ -517,6 +544,15 @@ fn handle_command_complete(server: &mut Server, message: &BytesMut) { CommandCompleteEffect::ArmDeclare => { server.cleanup_state.needs_cleanup_declare = true; } + CommandCompleteEffect::ArmListen => { + server.cleanup_state.needs_cleanup_listen = true; + } + CommandCompleteEffect::ArmPrepare => { + server.cleanup_state.needs_cleanup_prepare = true; + } + CommandCompleteEffect::ArmTemp => { + server.cleanup_state.needs_cleanup_temp = true; + } CommandCompleteEffect::DisarmSet => { server.cleanup_state.needs_cleanup_set = false; } @@ -530,6 +566,11 @@ fn handle_command_complete(server: &mut Server, message: &BytesMut) { CommandCompleteEffect::DisarmAll => { server.cleanup_state.reset(); drop_prepared_statement_cache_on_reset(server, "DISCARD ALL"); + // DISCARD ALL also resets startup parameters, which the backend never + // reports back. Drop them so the next checkout re-applies its own values. + if !server.resetting { + server.server_parameters.forget_untracked(); + } } } } @@ -858,6 +899,9 @@ where // EmptyQueryResponse // Response to Execute with an empty query string 'I' => { + if server.resetting { + server.last_sql_error = Some(("XX000".into(), "empty backend reset".into())); + } if server.is_async() { server.decrement_expected(); } @@ -913,10 +957,7 @@ mod tests { } #[test] - fn reset_tag_disarms_set_cleanup() { - // PostgreSQL emits the same `RESET\0` tag for `RESET ALL` and - // `RESET foo.bar`; both restore GUCs to their default so either one - // legitimately disarms pg_doorman's heuristic flag. + fn reset_tag_is_classified_for_cleanup_context() { assert_eq!( classify_command_complete(b"RESET\0"), CommandCompleteEffect::DisarmSet, @@ -965,12 +1006,118 @@ mod tests { ); } + #[test] + fn listen_tag_arms_listen_cleanup() { + assert_eq!( + classify_command_complete(b"LISTEN\0"), + CommandCompleteEffect::ArmListen, + ); + } + + #[test] + fn unlisten_tag_is_inert() { + // One tag covers both `UNLISTEN ch` and `UNLISTEN *`. A partial + // unsubscribe proves nothing about the remaining subscriptions, so + // UNLISTEN never disarms; the checkin UNLISTEN * is an idempotent no-op. + assert_eq!( + classify_command_complete(b"UNLISTEN\0"), + CommandCompleteEffect::None, + ); + } + + #[test] + fn sql_prepare_tag_arms_prepare_cleanup() { + // SQL PREPARE installs a named statement in the same server-side + // namespace as an extended-protocol Parse; the checkin DEALLOCATE ALL + // releases it and the pooler's text-keyed cache re-prepares on demand. + assert_eq!( + classify_command_complete(b"PREPARE\0"), + CommandCompleteEffect::ArmPrepare, + ); + } + + #[test] + fn temp_creating_tags_arm_temp_cleanup() { + // None of these tags distinguish temp from permanent objects; arming + // is intentional and DISCARD TEMP is a no-op for permanent ones. + // Current PostgreSQL answers CREATE TABLE AS / SELECT INTO with the + // inner query's tag, so these two only fire on older forks. + for tag in [ + &b"CREATE TABLE\0"[..], + b"CREATE TABLE AS\0", + b"SELECT INTO\0", + ] { + assert_eq!( + classify_command_complete(tag), + CommandCompleteEffect::ArmTemp, + "tag {:?} should arm temp cleanup", + std::str::from_utf8(tag).unwrap_or(""), + ); + } + } + + #[test] + fn select_into_inner_select_tag_is_inert() { + // `SELECT ... INTO TEMP` / `CREATE TEMP TABLE AS SELECT` on current + // PostgreSQL complete with the inner query's tag, same as a plain + // SELECT. Tracking it would fire on every read query, so these + // statements are documented as not detectable. + for tag in [&b"SELECT 1\0"[..], b"SELECT 42\0"] { + assert_eq!( + classify_command_complete(tag), + CommandCompleteEffect::None, + "tag {:?} should not influence cleanup", + std::str::from_utf8(tag).unwrap_or(""), + ); + } + } + + #[test] + fn untracked_ddl_tags_are_inert() { + // A permanent DROP is not proof that every temp object is gone, and a + // temp sequence (CREATE SEQUENCE) carries no distinguishable tag. + for tag in [ + &b"DROP TABLE\0"[..], + b"DROP TABLE 3\0", + b"CREATE SEQUENCE\0", + b"CREATE INDEX\0", + b"NOTIFY\0", + ] { + assert_eq!( + classify_command_complete(tag), + CommandCompleteEffect::None, + "tag {:?} should not influence cleanup", + std::str::from_utf8(tag).unwrap_or(""), + ); + } + } + + #[test] + fn create_table_prefix_does_not_confuse_classifier() { + // CREATE TABLE AS shares its prefix with CREATE TABLE; the comparison + // is full-length, so each tag must classify independently. + assert_eq!( + classify_command_complete(b"CREATE TABLE\0"), + CommandCompleteEffect::ArmTemp, + ); + assert_eq!( + classify_command_complete(b"CREATE TABLE AS\0"), + CommandCompleteEffect::ArmTemp, + ); + // Same prefix with an unrelated tail — inert. + assert_eq!( + classify_command_complete(b"CREATE TABLESPACE\0"), + CommandCompleteEffect::None, + ); + } + #[test] fn partial_discard_tags_are_inert() { // DISCARD PLANS drops the plan cache, DISCARD TEMP drops temp tables, // DISCARD SEQUENCES resets sequence caches. None of them revert SET - // state or drop prepared statements, so none should influence the - // cleanup flags on their own. + // state or drop prepared statements. A client DISCARD TEMP also does + // not disarm temp-cleanup: the redundant checkin DISCARD TEMP is an + // idempotent no-op. assert_eq!( classify_command_complete(b"DISCARD PLANS\0"), CommandCompleteEffect::None, diff --git a/src/server/server_backend.rs b/src/server/server_backend.rs index 6f50b5621..383ab0e2d 100644 --- a/src/server/server_backend.rs +++ b/src/server/server_backend.rs @@ -12,7 +12,7 @@ use lru::LruCache; use tokio::io::{AsyncReadExt, BufStream}; use crate::auth::scram_client::ScramSha256; -use crate::config::{get_config, tls, Address, BackendAuthMethod, User}; +use crate::config::{config_arc, get_config, tls, Address, BackendAuthMethod, CleanupMode, User}; use crate::errors::{Error, ServerIdentifier}; use crate::messages::PgErrorMsg; use crate::messages::{ @@ -109,7 +109,12 @@ pub struct Server { /// Configuration flag: if true, execute cleanup statements (RESET ALL, etc.) on dirty connections /// before returning them to the pool. If false, discard dirty connections instead. - cleanup_connections: bool, + cleanup_connections: CleanupMode, + + pub(crate) cleanup_server_query: Option, + // Backends that served client traffic in `always` mode are reset on checkin. + used_since_cleanup: bool, + pub(crate) resetting: bool, /// Configuration flag: if true, log when server parameters change for debugging purposes. pub(crate) log_client_parameter_status_changes: bool, @@ -200,7 +205,7 @@ impl Server { // new ReadyForQuery. self.last_sql_error = None; - self.send_and_flush(&query).await?; + protocol_io::send_and_flush(self, &query).await?; let mut noop = tokio::io::sink(); loop { @@ -209,6 +214,9 @@ impl Server { Err(err) => return Err(err), } + if self.resetting && self.in_copy_mode { + return Err(Error::QueryError("backend reset entered COPY mode".into())); + } if !self.data_available { break; } @@ -231,7 +239,7 @@ impl Server { pub async fn check_alive(&mut self, timeout: Duration) -> Result<(), Error> { let query = simple_query(";"); - self.send_and_flush_timeout(&query, timeout).await?; + protocol_io::send_and_flush_timeout(self, &query, timeout).await?; let mut noop = tokio::io::sink(); loop { @@ -367,10 +375,12 @@ impl Server { messages: &BytesMut, duration: Duration, ) -> Result<(), Error> { + self.used_since_cleanup |= self.cleanup_connections == CleanupMode::Always; protocol_io::send_and_flush_timeout(self, messages, duration).await } pub async fn send_and_flush(&mut self, messages: &BytesMut) -> Result<(), Error> { + self.used_since_cleanup |= self.cleanup_connections == CleanupMode::Always; protocol_io::send_and_flush(self, messages).await } @@ -397,12 +407,36 @@ impl Server { /// Perform any necessary cleanup before putting the server /// connection back in the pool pub async fn checkin_cleanup(&mut self) -> Result<(), Error> { - self.pending_large_message = None; - if self.in_copy_mode() { + let result = self.checkin_cleanup_inner().await; + if let Err(err) = &result { + self.bad = true; + // Protocol guards already reported their reason through mark_bad. A cleanup + // query failure has no other report, so this line covers both cases. warn!( - "[{}@{}] server returned in copy-mode pid={}", + "[{}@{}] server cleanup failed, retiring backend pid={}: {err}", self.address.username, self.address.pool_name, self.process_id ); + } + result + } + + /// A maintenance failure must not replace an already completed query result. + /// Protocol errors detected before cleanup still propagate to the client. + pub(crate) async fn cleanup_after_response(&mut self) -> Result<(), Error> { + let response_complete = !self.in_transaction + && !self.in_copy_mode + && !self.data_available + && self.buffer.is_empty() + && !self.is_async(); + match self.checkin_cleanup().await { + Err(_) if response_complete => Ok(()), + result => result, + } + } + + async fn checkin_cleanup_inner(&mut self) -> Result<(), Error> { + self.pending_large_message = None; + if self.in_copy_mode() { self.mark_bad("returned in copy-mode"); return Err(Error::ProtocolSyncError(format!( "Protocol synchronization error: Server {} (database: {}, user: {}) was returned to the pool while still in COPY mode. This may indicate a client disconnected during a COPY operation.", @@ -410,10 +444,6 @@ impl Server { ))); } if self.is_data_available() { - warn!( - "[{}@{}] server returned with data available pid={}", - self.address.username, self.address.pool_name, self.process_id - ); self.mark_bad("returned with data available"); return Err(Error::ProtocolSyncError(format!( "Protocol synchronization error: Server {} (database: {}, user: {}) was returned to the pool while still having data available. This may indicate a client disconnected before receiving all query results.", @@ -421,16 +451,61 @@ impl Server { ))); } if !self.buffer.is_empty() { - warn!( - "[{}@{}] server returned with non-empty buffer pid={}", - self.address.username, self.address.pool_name, self.process_id - ); - self.mark_bad("returned with not-empty buffer"); + self.mark_bad("returned with non-empty buffer"); return Err(Error::ProtocolSyncError(format!( "Protocol synchronization error: Server {} (database: {}, user: {}) was returned to the pool with a non-empty buffer. This may indicate a client disconnected before the server response was fully processed.", self.address.host, self.address.database, self.address.username ))); } + // The pool retires bad backends; do not send more SQL to them. + if self.bad { + return Ok(()); + } + if self.custom_cleanup_enabled() { + let needs_cleanup = (self.cleanup_connections == CleanupMode::Always + && self.used_since_cleanup) + || self.cleanup_state.needs_cleanup() + || self.has_pending_cache_entries + || !self.deferred_eviction_closes.is_empty(); + let query = self + .cleanup_server_query + .as_ref() + .filter(|_| needs_cleanup) + .cloned(); + if query.is_some() || self.in_transaction() { + return self.reset_with_query(query.as_deref()).await; + } + return Ok(()); + } + + // off: only the ROLLBACK of a transaction the client left open. + // adaptive: that ROLLBACK plus the built-in session reset, and only when + // pg_doorman saw the session state change. + let needs_rollback = self.in_transaction(); + let needs_session_reset = self.cleanup_connections != CleanupMode::Off + && (self.cleanup_state.needs_cleanup() + || self.has_pending_cache_entries + || !self.deferred_eviction_closes.is_empty()); + if !needs_rollback && !needs_session_reset { + return Ok(()); + } + self.bad = true; + self.resetting = true; + self.set_async_mode(false); + self.reset_expected_responses(); + let result = self.cleanup_builtin(needs_session_reset).await; + self.resetting = false; + self.address.stats.server_cleanup(result.is_ok()); + if result.is_ok() { + self.bad = false; + } + result + } + + /// Built-in cleanup. The ROLLBACK runs in every mode. The RESET sequence runs + /// only when `session_reset` is set, so `CleanupMode::Off` reaches here with an + /// open transaction and the ROLLBACK is the whole cleanup. + async fn cleanup_builtin(&mut self, session_reset: bool) -> Result<(), Error> { // Client disconnected with an open transaction on the server connection. // Pgbouncer behavior is to close the server connection but that can cause // server connection thrashing if clients repeatedly do this. @@ -464,14 +539,15 @@ impl Server { // to avoid leaking state between clients. For performance reasons we only // send `RESET ALL` if we think the session is altered instead of just sending // it before each checkin. - if self.cleanup_state.needs_cleanup() && self.cleanup_connections { + if session_reset { info!( "[{}@{}] session state cleanup pid={}: {}", self.address.username, self.address.pool_name, self.process_id, self.cleanup_state ); let mut reset_string = String::from("RESET ROLE;"); + let reset_all = self.cleanup_state.needs_cleanup_set; - if self.cleanup_state.needs_cleanup_set { + if reset_all { reset_string.push_str("RESET ALL;"); }; @@ -483,18 +559,25 @@ impl Server { reset_string.push_str("CLOSE ALL;"); }; + if self.cleanup_state.needs_cleanup_listen { + reset_string.push_str("UNLISTEN *;"); + }; + + // Advisory locks carry no command tag to track; release them in the + // same round trip whenever the cleanup batch is sent anyway. The + // ROLLBACK above releases transaction-level locks only, so a + // session-level advisory lock would otherwise outlive the client. + reset_string.push_str("SELECT pg_advisory_unlock_all();"); + + if self.cleanup_state.needs_cleanup_temp { + reset_string.push_str("DISCARD TEMP;"); + }; + self.small_simple_query(&reset_string).await?; - if self.cleanup_state.needs_cleanup_prepare { - // flush prepared. - self.registering_prepared_statement.clear(); - if self.prepared_statement_cache.is_some() { - let cache_size = self.prepared_statement_cache.as_ref().unwrap().len(); - info!( - "[{}@{}] clearing prepared statement cache pid={}: session state reset ({} entries)", - self.address.username, self.address.pool_name, self.process_id, cache_size - ); - self.prepared_statement_cache.as_mut().unwrap().clear(); - } + if reset_all { + self.server_parameters.forget_untracked(); + } else { + self.server_parameters.remove_param("role"); } self.cleanup_state.reset(); } @@ -503,6 +586,62 @@ impl Server { Ok(()) } + pub(crate) fn custom_cleanup_enabled(&self) -> bool { + self.cleanup_connections != CleanupMode::Off && self.cleanup_server_query.is_some() + } + + async fn reset_with_query(&mut self, query: Option<&str>) -> Result<(), Error> { + // Failure or cancellation must never return a partially reset backend. + self.bad = true; + self.resetting = true; + self.set_async_mode(false); + self.reset_expected_responses(); + let config = config_arc(); + let timeout = config + .pools + .get(&self.address.pool_name) + .map(|pool| pool.effective_connect_timeout(&config.general)) + .unwrap_or(config.general.connect_timeout.as_std()); + let result = tokio::time::timeout(timeout, async { + if self.in_transaction { + self.small_simple_query("ROLLBACK").await?; + if self.in_transaction { + return Err(Error::QueryError( + "backend rollback did not finish Idle".into(), + )); + } + } + if let Some(query) = query { + self.small_simple_query(query).await?; + } + if self.in_transaction || self.in_copy_mode || self.data_available { + return Err(Error::QueryError( + "backend reset did not finish Idle".into(), + )); + } + Ok(()) + }) + .await + .map_err(|_| Error::QueryError("backend reset timed out".into())) + .and_then(|result| result); + self.resetting = false; + self.address.stats.server_cleanup(result.is_ok()); + result?; + if query.is_some() { + self.registering_prepared_statement.clear(); + self.has_pending_cache_entries = false; + self.deferred_eviction_closes.clear(); + if let Some(cache) = self.prepared_statement_cache.as_mut() { + cache.clear(); + } + self.server_parameters.forget_untracked(); + self.cleanup_state.reset(); + } + self.used_since_cleanup = false; + self.bad = false; + Ok(()) + } + /// We don't buffer all of server responses, e.g. COPY OUT produces too much data. /// The client is responsible to call `self.recv()` while this method returns true. #[inline(always)] @@ -803,7 +942,8 @@ impl Server { database: &str, client_server_map: ClientServerMap, stats: Arc, - cleanup_connections: bool, + cleanup_connections: CleanupMode, + cleanup_server_query: Option, log_client_parameter_status_changes: bool, server_prepared_statement_cache_size: usize, application_name: String, @@ -1108,6 +1248,9 @@ impl Server { application_name, last_activity: SystemTime::now(), cleanup_connections, + cleanup_server_query, + used_since_cleanup: false, + resetting: false, log_client_parameter_status_changes, prepared_statement_cache: match server_prepared_statement_cache_size { 0 => None, diff --git a/src/stats/address.rs b/src/stats/address.rs index 02cd0d8b9..3831cc422 100644 --- a/src/stats/address.rs +++ b/src/stats/address.rs @@ -104,6 +104,10 @@ pub struct AddressStats { /// new shard entry under a brief write lock. pub errors_by_sqlstate: DashMap, + /// Cleanup attempt counters shared by every backend of this address. + pub(crate) server_cleanup_ok: AtomicU64, + pub(crate) server_cleanup_error: AtomicU64, + /// Process-unique identifier for this `AddressStats` instance. /// Every `Default::default()` mints a fresh value from a static /// monotonic counter. The Prometheus scrape path passes this into @@ -140,6 +144,8 @@ impl Default for AddressStats { wait_histogram: Mutex::new(new_histogram()), p95_xact_time_us: AtomicU64::new(0), errors_by_sqlstate: DashMap::new(), + server_cleanup_ok: AtomicU64::new(0), + server_cleanup_error: AtomicU64::new(0), generation: next_address_stats_generation(), } } @@ -232,6 +238,17 @@ impl IntoIterator for &AddressStats { } impl AddressStats { + /// Counts a cleanup attempt that reached the backend; skipped cleanup is not counted. + #[inline] + pub fn server_cleanup(&self, success: bool) { + let counter = if success { + &self.server_cleanup_ok + } else { + &self.server_cleanup_error + }; + counter.fetch_add(1, Ordering::Relaxed); + } + /// Increments the transaction count in both total and current statistics. /// /// This method is called whenever a new transaction is started. diff --git a/src/web/metrics/metrics.rs b/src/web/metrics/metrics.rs index 1240f2823..b1ac30060 100644 --- a/src/web/metrics/metrics.rs +++ b/src/web/metrics/metrics.rs @@ -46,6 +46,7 @@ pub fn update_metrics() { update_pool_metrics(); update_pool_errors_metrics(); + update_server_cleanup_metrics(); update_server_metrics(); update_auth_query_metrics(); update_coordinator_metrics(); @@ -930,6 +931,42 @@ type PoolErrorsKey = (String, String, &'static str); static POOL_ERRORS_PREV: Lazy> = Lazy::new(CounterDeltaTracker::new); +type ServerCleanupKey = (String, String, &'static str); +static SERVER_CLEANUP_PREV: Lazy> = + Lazy::new(CounterDeltaTracker::new); + +fn update_server_cleanup_metrics() { + let mut current_keys = std::collections::HashSet::new(); + for (identifier, pool) in crate::pool::get_all_pools().iter() { + let stats = &pool.address().stats; + for (result, source) in [ + ("ok", &stats.server_cleanup_ok), + ("error", &stats.server_cleanup_error), + ] { + let key = (identifier.user.clone(), identifier.db.clone(), result); + let counter = super::SERVER_CLEANUP_TOTAL.with_label_values(&[ + identifier.user.as_str(), + identifier.db.as_str(), + result, + ]); + SERVER_CLEANUP_PREV.observe( + &counter, + key.clone(), + stats.generation, + source.load(Ordering::Relaxed), + ); + current_keys.insert(key); + } + } + for stale in SERVER_CLEANUP_PREV.drain_stale(¤t_keys) { + let _ = super::SERVER_CLEANUP_TOTAL.remove_label_values(&[ + stale.0.as_str(), + stale.1.as_str(), + stale.2, + ]); + } +} + fn update_pool_errors_metrics() { use crate::pool::get_all_pools; diff --git a/src/web/metrics/mod.rs b/src/web/metrics/mod.rs index 7dc450b0e..15c7d4072 100644 --- a/src/web/metrics/mod.rs +++ b/src/web/metrics/mod.rs @@ -249,6 +249,24 @@ pub(crate) static SHOW_POOLS_ERRORS_TOTAL: Lazy = Lazy::new(|| { counter }); +/// Counted per user and database in a static registry: retiring a backend does not reset the totals. +pub(crate) static SERVER_CLEANUP_TOTAL: Lazy = Lazy::new(|| { + let counter = IntCounterVec::new( + Opts::new( + "pg_doorman_server_cleanup_total", + "Backend cleanup attempts per pool and user. Counted once when pg_doorman sent cleanup \ + SQL to the backend: the built-in sequence in 'adaptive', cleanup_server_query in \ + 'always', or a ROLLBACK of a transaction the client left open. In 'off' only that \ + ROLLBACK is counted. Result is 'ok' or 'error'; 'error' means the backend was retired. \ + Checkins that needed no cleanup are not counted. The 'database' label is the pool name.", + ), + &["user", "database", "result"], + ) + .unwrap(); + REGISTRY.register(Box::new(counter.clone())).unwrap(); + counter +}); + /// DEPRECATED: monotonic byte counter shipped as a Gauge. Prefer /// `pg_doorman_pools_bytes_total`. Scheduled for removal in 3.10. pub(crate) static SHOW_POOLS_BYTES: Lazy = Lazy::new(|| { diff --git a/tests/Makefile b/tests/Makefile index ba280f436..9339b98c7 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,4 +1,4 @@ -.PHONY: help local-build pull shell tests-build test-all test-bdd test-rust test-go test-python test-nodejs test-dotnet test-php clean +.PHONY: help local-build pull shell tests-build test-all test-bdd test-greengage test-rust test-go test-python test-nodejs test-dotnet test-php clean REGISTRY ?= ghcr.io IMAGE_TAG ?= latest @@ -51,6 +51,9 @@ else @$(TESTS_DIR)nix/run-tests.sh bdd endif +test-greengage: ## Run cleanup BDD against a disposable Greengage cluster + @$(TESTS_DIR)nix/run-tests.sh greengage $(if $(TAGS),"$(TAGS)") + test-go: ## Run Go client tests @$(TESTS_DIR)nix/run-tests.sh test-go @@ -82,4 +85,4 @@ clean: ## Remove cached Docker volumes @docker volume rm pg_doorman_cargo_cache pg_doorman_cargo_git \ pg_doorman_go_cache pg_doorman_go_build \ pg_doorman_npm_cache pg_doorman_dotnet pg_doorman_nuget 2>/dev/null || true - @echo "Cache cleaned!" \ No newline at end of file + @echo "Cache cleaned!" diff --git a/tests/bdd/extended/mod.rs b/tests/bdd/extended/mod.rs index da8840cf4..42ccf3411 100644 --- a/tests/bdd/extended/mod.rs +++ b/tests/bdd/extended/mod.rs @@ -4,4 +4,5 @@ mod backend_pid; mod cancel_protocol; mod dual_connection; pub mod helpers; +mod notifications; mod session_management; diff --git a/tests/bdd/extended/notifications.rs b/tests/bdd/extended/notifications.rs new file mode 100644 index 000000000..f9121fcdd --- /dev/null +++ b/tests/bdd/extended/notifications.rs @@ -0,0 +1,59 @@ +use crate::world::DoormanWorld; +use cucumber::then; + +/// NotificationResponse body: Int32 backend pid, CString channel, CString payload. +fn parse_notification(body: &[u8]) -> Option<(i32, String, String)> { + if body.len() < 5 { + return None; + } + let pid = i32::from_be_bytes([body[0], body[1], body[2], body[3]]); + let rest = &body[4..]; + let channel_end = rest.iter().position(|&b| b == 0)?; + let channel = String::from_utf8_lossy(&rest[..channel_end]).to_string(); + let payload = &rest[channel_end + 1..]; + let payload_end = payload + .iter() + .position(|&b| b == 0) + .unwrap_or(payload.len()); + let payload = String::from_utf8_lossy(&payload[..payload_end]).to_string(); + Some((pid, channel, payload)) +} + +fn notification_channels(world: &DoormanWorld, session: &str) -> Vec { + world + .session_messages + .get(session) + .unwrap_or_else(|| panic!("Session {session}: no stored response")) + .iter() + .filter(|(tag, _)| *tag == 'A') + .filter_map(|(_, body)| parse_notification(body).map(|(_, channel, _)| channel)) + .collect() +} + +#[then(regex = r#"^session "([^"]+)" should receive NotificationResponse for channel "([^"]+)"$"#)] +pub async fn should_receive_notification( + world: &mut DoormanWorld, + session: String, + channel: String, +) { + let channels = notification_channels(world, &session); + assert!( + channels.iter().any(|c| *c == channel), + "Session {session}: expected NotificationResponse for channel {channel:?}, got {channels:?}" + ); +} + +#[then( + regex = r#"^session "([^"]+)" should not receive NotificationResponse for channel "([^"]+)"$"# +)] +pub async fn should_not_receive_notification( + world: &mut DoormanWorld, + session: String, + channel: String, +) { + let channels = notification_channels(world, &session); + assert!( + !channels.iter().any(|c| *c == channel), + "Session {session}: unexpected NotificationResponse for channel {channel:?}, got {channels:?}" + ); +} diff --git a/tests/bdd/extended/session_management.rs b/tests/bdd/extended/session_management.rs index 1fb91ac33..c5ed9d4cf 100644 --- a/tests/bdd/extended/session_management.rs +++ b/tests/bdd/extended/session_management.rs @@ -777,6 +777,69 @@ pub async fn send_flush_to_session(world: &mut DoormanWorld, session_name: Strin conn.send_flush().await.expect("Failed to send Flush"); } +// Flush has no ReadyForQuery. Read an explicit response barrier before the next send. +#[when(regex = r#"^we read backend messages "([12DCETZ]+)" from session "([^"]+)"$"#)] +pub async fn read_backend_messages(world: &mut DoormanWorld, tags: String, session_name: String) { + let conn = super::helpers::get_session(&mut world.named_sessions, &session_name); + let messages = timeout(Duration::from_secs(5), async { + let mut messages = Vec::new(); + for tag in tags.chars() { + let message = conn.read_message().await.expect("Failed to read response"); + assert_eq!(message.0, tag, "Unexpected backend message: {message:?}"); + messages.push(message); + } + messages + }) + .await + .expect("Timed out reading backend response barrier"); + world.session_messages.insert(session_name, messages); +} + +#[when(regex = r#"^we store backend_pid from last response of session "([^"]+)"$"#)] +pub async fn store_response_backend_pid(world: &mut DoormanWorld, session_name: String) { + let (_, data) = world.session_messages[&session_name] + .iter() + .find(|(tag, _)| *tag == 'D') + .expect("No DataRow containing backend PID"); + let pid = super::helpers::parse_datarow_fields(data)[0] + .parse::() + .expect("First DataRow field must be a backend PID"); + println!("Session '{session_name}' backend_pid: {pid}"); + world + .vars + .insert(format!("{session_name}_pid"), pid.to_string()); + world.session_backend_pids.insert(session_name, pid); +} + +#[when(regex = r#"^we store backend_pid from last response of session "([^"]+)" as "([^"]+)"$"#)] +pub async fn store_named_backend_pid_from_response( + world: &mut DoormanWorld, + session_name: String, + pid_name: String, +) { + store_response_backend_pid(world, session_name.clone()).await; + let pid = world.session_backend_pids[&session_name]; + world + .named_backend_pids + .insert((session_name, pid_name), pid); +} + +#[then(regex = r#"^session "([^"]+)" should receive exactly one DataRow "([^"]+)"$"#)] +pub async fn session_should_receive_exact_row( + world: &mut DoormanWorld, + session_name: String, + expected: String, +) { + let rows: Vec<_> = world.session_messages[&session_name] + .iter() + .filter(|(tag, _)| *tag == 'D') + .map(|(_, data)| super::helpers::parse_datarow_fields(data)) + .collect(); + let expected = world.replace_placeholders(&expected); + let fields: Vec<_> = expected.split('|').collect(); + assert_eq!(rows, vec![fields], "Unexpected DataRows for {session_name}"); +} + #[when(regex = r#"^we send Sync to session "([^"]+)"$"#)] #[then(regex = r#"^we send Sync to session "([^"]+)"$"#)] pub async fn send_sync_to_session(world: &mut DoormanWorld, session_name: String) { diff --git a/tests/bdd/features/cleanup-check-query-write-failure.feature b/tests/bdd/features/cleanup-check-query-write-failure.feature new file mode 100644 index 000000000..237049194 --- /dev/null +++ b/tests/bdd/features/cleanup-check-query-write-failure.feature @@ -0,0 +1,50 @@ +@rust @rust-3 @cleanup-review @cleanup-review-check-query-write-failure +Feature: Cleanup runs even when the check query response cannot be written to the client + recycle() does not clean backends, so the pooler check query path must clean the + backend itself even when the response write to the client fails. + + Background: + Given PostgreSQL started with options "-c log_statement=all -c logging_collector=off" and pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pooler_check_query: "SELECT 7, pg_sleep(1)" + cleanup_server_connections: always + cleanup_server_query: "SELECT set_config('doorman.cleanup_ran', 'yes', false)" + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + + Scenario: A failed client write after a pooler check query still cleans the backend + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store response + And we store backend_pid from last response of session "one" + # The first response already ran one cleanup in always mode; truncate the log + # so the marker count below covers only the check query response. + And we truncate PostgreSQL log + # Send the check query without reading the response and RST the socket while the + # backend is inside pg_sleep(1): the response write to the client must fail. + And we send SimpleQuery "SELECT 7, pg_sleep(1)" to session "one" without waiting + And we abort TCP connection with RST for session "one" + And we sleep 1500ms + Then PostgreSQL log should contain exactly 1 occurrences of "set_config('doorman.cleanup_ran'" + # The backend was cleaned, not retired, so the next checkout reuses the same pid + # and sees the cleanup marker. + When we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid(), current_setting('doorman.cleanup_ran')" to session "next" and store response + Then session "next" should receive exactly one DataRow "${one_pid}|yes" + And session "next" should receive ReadyForQuery "I" diff --git a/tests/bdd/features/cleanup-config-review.feature b/tests/bdd/features/cleanup-config-review.feature new file mode 100644 index 000000000..bcf37e52d --- /dev/null +++ b/tests/bdd/features/cleanup-config-review.feature @@ -0,0 +1,177 @@ +@rust @rust-3 @cleanup-review @cleanup-config-review +Feature: Cleanup policy configuration errors + Invalid cleanup policies identify the setting and its accepted values. + + Scenario Outline: Invalid cleanup policy in + When I run shell command: + """ + python3 - <<'PY' + import subprocess + import sys + import tempfile + + value = '' + scope = '' + if '' == 'toml': + content = '[general]\nadmin_username = "admin"\nadmin_password = "admin"\n' + if scope == 'pool': + content += '[pools.example_db]\n' + content += f'cleanup_server_connections = {value}\n' + else: + content = 'general:\n admin_username: admin\n admin_password: admin\n' + if scope == 'pool': + content += 'pools:\n example_db:\n' + indent = ' ' if scope == 'pool' else ' ' + content += f'{indent}cleanup_server_connections: {value}\n' + + with tempfile.NamedTemporaryFile(mode='w', suffix='.') as config: + config.write(content) + config.flush() + result = subprocess.run( + ['${DOORMAN_BINARY}', '-t', config.name], + capture_output=True, text=True, timeout=10, + ) + print(result.stdout + result.stderr) + sys.exit(result.returncode) + PY + """ + Then the command should fail + And the command output should contain "cleanup_server_connections" + And the command output should contain "expected a boolean or one of: off, adaptive, always" + + Examples: + | format | scope | value | + | toml | general | 1 | + | toml | general | [true] | + | toml | general | "auto" | + | toml | pool | 1 | + | toml | pool | [true] | + | toml | pool | "auto" | + | yaml | general | 1 | + | yaml | general | [true] | + | yaml | general | "auto" | + | yaml | pool | 1 | + | yaml | pool | [true] | + | yaml | pool | "auto" | + + @cleanup-review @cleanup-config-review-pairs + Scenario Outline: Cleanup policy is rejected in + When I run shell command: + """ + python3 - <<'PY' + import subprocess + import sys + import tempfile + + fmt = '' + + def block(scope): + mode = '' if scope == 'general' else '' + query = '' if scope == 'general' else '' + indent = ' ' if scope == 'general' else ' ' + settings = '' + if mode: + settings += f'cleanup_server_connections = {mode}\n' if fmt == 'toml' \ + else f'{indent}cleanup_server_connections: {mode}\n' + if query: + settings += f'cleanup_server_query = "{query}"\n' if fmt == 'toml' \ + else f'{indent}cleanup_server_query: "{query}"\n' + return settings or f'{indent}# inherit defaults\n' + + if fmt == 'toml': + content = '[general]\nadmin_username = "admin"\nadmin_password = "admin"\n' + content += block('general') + content += '[pools.example_db]\n' + content += block('pool') + else: + content = 'general:\n admin_username: admin\n admin_password: admin\n' + content += block('general') + content += 'pools:\n example_db:\n pool_mode: session\n' + content += block('pool') + + with tempfile.NamedTemporaryFile(mode='w', suffix='.' + fmt) as config: + config.write(content) + config.flush() + result = subprocess.run( + ['${DOORMAN_BINARY}', '-t', config.name], + capture_output=True, text=True, timeout=10, + ) + print(result.stdout + result.stderr) + sys.exit(result.returncode) + PY + """ + Then the command should fail + And the command output should contain "pools.example_db" + And the command output should contain "" + + Examples: + | format | case | general_mode | general_query | pool_mode | pool_query | hint | + | toml | adaptive general with a query | "adaptive" | DISCARD ALL | | | requires cleanup_server_connections = always | + | toml | legacy true general with a query | true | DISCARD ALL | | | requires cleanup_server_connections = always | + | toml | adaptive pool with its own query | | | "adaptive" | DISCARD ALL | requires cleanup_server_connections = always | + | toml | legacy true pool with its own query | | | true | DISCARD ALL | requires cleanup_server_connections = always | + | toml | pool narrowing general always to adaptive | "always" | DISCARD ALL | "adaptive" | | requires cleanup_server_connections = always | + | toml | always general without a query | "always" | | | | requires cleanup_server_query | + | toml | always pool without a query | | | "always" | | requires cleanup_server_query | + | yaml | adaptive general with a query | adaptive | DISCARD ALL | | | requires cleanup_server_connections = always | + | yaml | adaptive pool with its own query | | | adaptive | DISCARD ALL | requires cleanup_server_connections = always | + | yaml | pool narrowing general always to adaptive | always | DISCARD ALL | adaptive | | requires cleanup_server_connections = always | + | yaml | always general without a query | always | | | | requires cleanup_server_query | + | yaml | always pool without a query | | | always | | requires cleanup_server_query | + + @cleanup-review @cleanup-config-review-pairs + Scenario Outline: Cleanup policy in + When I run shell command: + """ + python3 - <<'PY' + import subprocess + import sys + import tempfile + + fmt = '' + mode = '' + query = '' + override = '' + if fmt == 'toml': + content = '[general]\nadmin_username = "admin"\nadmin_password = "admin"\n' + content += f'cleanup_server_connections = {mode}\n' + if query: + content += f'cleanup_server_query = "{query}"\n' + content += '[pools.example_db]\n' + if override: + content += f'cleanup_server_connections = {override}\n' + else: + content = 'general:\n admin_username: admin\n admin_password: admin\n' + content += f' cleanup_server_connections: {mode}\n' + if query: + content += f' cleanup_server_query: "{query}"\n' + content += 'pools:\n example_db:\n pool_mode: session\n' + if override: + content += f' cleanup_server_connections: {override}\n' + + with tempfile.NamedTemporaryFile(mode='w', suffix='.' + fmt) as config: + config.write(content) + config.flush() + result = subprocess.run( + ['${DOORMAN_BINARY}', '-t', config.name], + capture_output=True, text=True, timeout=10, + ) + print(result.stdout + result.stderr) + sys.exit(result.returncode) + PY + """ + Then the command should succeed + And the command output should contain "test is successful" + + Examples: + | format | case | mode | query | pool_override | + | toml | adaptive without a query | "adaptive" | | | + | toml | always with a query | "always" | DISCARD ALL | | + | toml | off with a query | "off" | DISCARD ALL | | + | toml | legacy false with a query | false | DISCARD ALL | | + | toml | always pool narrowed to off keeps the query | "always" | DISCARD ALL | "off" | + | yaml | adaptive without a query | adaptive | | | + | yaml | always with a query | always | DISCARD ALL | | + | yaml | off with a query | off | DISCARD ALL | | + | yaml | legacy false with a query | false | DISCARD ALL | | + | yaml | always pool narrowed to off keeps the query | always | DISCARD ALL | off | diff --git a/tests/bdd/features/cleanup-listen.feature b/tests/bdd/features/cleanup-listen.feature new file mode 100644 index 000000000..5db354dd3 --- /dev/null +++ b/tests/bdd/features/cleanup-listen.feature @@ -0,0 +1,128 @@ +@rust @rust-3 @cleanup-review @cleanup-listen +Feature: Adaptive cleanup removes LISTEN subscriptions left by a client + A backend released to the pool keeps its LISTEN subscriptions. While the + backend is pooled, nobody reads its socket, so NotificationResponse + messages queue up and get delivered to whichever client checks the + backend out next. The built-in cleanup must send UNLISTEN * at checkin + when a LISTEN was tracked. + + Background: + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + web: + enabled: true + host: "127.0.0.1" + port: 9129 + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + example_db_session: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + server_database: "example_db" + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + + @cleanup-listen-subscription + Scenario: A LISTEN subscription does not survive a checkin + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "LISTEN leak_ch" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + And we send SimpleQuery "SELECT 'leak_ch' IN (SELECT pg_listening_channels())" to session "next" and store response + Then session "next" should receive DataRow with "f" + + @cleanup-listen-notification + Scenario: A notification for a released subscription does not reach the next client + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "LISTEN leak_ch" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT 1" to session "next" + And we create session "notifier" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "NOTIFY leak_ch" to session "notifier" + And we send SimpleQuery "SELECT 2" to session "next" and store response + Then session "next" should receive DataRow with "2" + And session "next" should not receive NotificationResponse for channel "leak_ch" + + @cleanup-listen-partial-unlisten + Scenario: A partial client UNLISTEN does not suppress the checkin UNLISTEN + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "LISTEN ch_one" to session "old" + And we send SimpleQuery "LISTEN ch_two" to session "old" + # The UNLISTEN tag is shared by UNLISTEN ch and UNLISTEN *, so a partial + # unsubscribe cannot disarm the flag: ch_two is still subscribed. + And we send SimpleQuery "UNLISTEN ch_one" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + And we send SimpleQuery "SELECT count(*) FROM pg_listening_channels()" to session "next" and store response + Then session "next" should receive DataRow with "0" + + @cleanup-listen-discard-all + Scenario: A client DISCARD ALL suppresses the checkin UNLISTEN entirely + # Session pooling has a single checkin at disconnect, so LISTEN and the + # client's DISCARD ALL share one lease. (Transaction pooling checks the + # backend in after every query: the LISTEN cleanup already ran before + # the client's DISCARD ALL could matter.) + # The counter label is the pool name, so this reads the session pool + # the client actually used. + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "LISTEN suppress_ch" to session "old" + And we send SimpleQuery "DISCARD ALL" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "SELECT count(*) FROM pg_listening_channels()" to session "next" and store response + Then session "next" should receive DataRow with "0" + When I run shell command: + """ + python3 - <<'PY' + import time + import urllib.request + deadline = time.monotonic() + 5 + while True: + body = urllib.request.urlopen('http://127.0.0.1:9129/metrics', timeout=2).read().decode() + lines = body.splitlines() + ok = 'pg_doorman_server_cleanup_total{database="example_db_session",result="ok",user="example_user_1"} 0' in lines + err = 'pg_doorman_server_cleanup_total{database="example_db_session",result="error",user="example_user_1"} 0' in lines + if ok and err: + break + assert time.monotonic() < deadline, body + time.sleep(0.05) + PY + """ + Then the command should succeed + + @cleanup-listen-session-mode + Scenario: A session mode client keeps receiving notifications + When we create session "sub" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "LISTEN ping_ch" to session "sub" + And we create session "notifier" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "NOTIFY ping_ch, 'hello'" to session "notifier" + # The session mode client holds the backend; its next query result is + # preceded by the queued NotificationResponse. + And we send SimpleQuery "SELECT 1" to session "sub" and store response + Then session "sub" should receive DataRow with "1" + And session "sub" should receive NotificationResponse for channel "ping_ch" diff --git a/tests/bdd/features/cleanup-observability.feature b/tests/bdd/features/cleanup-observability.feature new file mode 100644 index 000000000..1652aa060 --- /dev/null +++ b/tests/bdd/features/cleanup-observability.feature @@ -0,0 +1,135 @@ +@rust @rust-3 @cleanup-review @cleanup-observability +Feature: Backend cleanup observability + + Background: + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + + @cleanup-review-metrics + Scenario Outline: Count completed cleanup attempts without counting idle checkins or scrapes + Given pg_doorman log capture enabled + And pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + cleanup_server_connections: + + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + web: + enabled: true + host: "127.0.0.1" + port: 9129 + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "" to session "one" + # Session pooling has not released the backend yet, so there was no cleanup. + When I run shell command: + """ + python3 - <<'PY' + import urllib.request + body = urllib.request.urlopen('http://127.0.0.1:9129/metrics', timeout=2).read().decode() + for result in ('ok', 'error'): + expected = 'pg_doorman_server_cleanup_total{database="example_db",result="' + result + '",user="example_user_1"} 0' + assert expected in body.splitlines(), body + PY + """ + Then the command should succeed + When we close session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT 1" to session "next" + # Checkout of the next lease waits for the previous checkin and must not + # introduce another cleanup attempt. Its session stays open during scrapes. + # Poll the observable completion of asynchronous session checkin, then scrape + # again to catch counters that accidentally increase during collection. + And I run shell command: + """ + python3 - <<'PY' + import time + import urllib.request + expected = {'ok': , 'error': } + deadline = time.monotonic() + 5 + def snapshot(): + body = urllib.request.urlopen('http://127.0.0.1:9129/metrics', timeout=2).read().decode() + lines = body.splitlines() + return all('pg_doorman_server_cleanup_total{database="example_db",result="' + result + '",user="example_user_1"} ' + str(count) in lines for result, count in expected.items()), body + while True: + matches, body = snapshot() + if matches: + break + assert time.monotonic() < deadline, body + time.sleep(0.05) + for _ in range(3): + matches, body = snapshot() + assert matches, body + PY + """ + Then the command should succeed + And pg_doorman log "[example_user_1@example_db] server cleanup failed, retiring backend" + + Examples: + | mode | query_config | client_query | ok | error | log_expectation | + | always | cleanup_server_query: "DISCARD ALL" | SET work_mem = '96MB' | 1 | 0 | does not contain | + | always | cleanup_server_query: "DISCARD ALL" | SELECT 1 | 1 | 0 | does not contain | + | adaptive | | SELECT 1 | 0 | 0 | does not contain | + | off | cleanup_server_query: "DISCARD ALL" | SET work_mem = '96MB' | 0 | 0 | does not contain | + | always | cleanup_server_query: "SELECT 1 / 0" | SET work_mem = '96MB' | 0 | 1 | contains | + | always | cleanup_server_query: "/* empty reset */" | SET work_mem = '96MB' | 0 | 1 | contains | + | always | cleanup_server_query: "SELECT pg_sleep(5)" | SET work_mem = '96MB' | 0 | 1 | contains | + | always | cleanup_server_query: "DISCARD ALL" | BEGIN; SELECT 1 | 1 | 0 | does not contain | + | adaptive | | SET work_mem = '96MB' | 1 | 0 | does not contain | + | adaptive | | BEGIN; SELECT 1 | 1 | 0 | does not contain | + + @cleanup-review-reset-logs + Scenario Outline: Routine configured cleanup does not warn when clearing the prepared cache + Given pg_doorman log capture enabled + And pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + prepared_statements: true + cleanup_server_connections: always + cleanup_server_query: "" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT 1" to session "one" + And we send Parse "cached" with query "SELECT $1::int + 1" to session "one" + And we send Bind "" to "cached" with params "10" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "11" + And session "one" should receive ReadyForQuery "I" + When we send Bind "" to "cached" with params "11" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "12" + And session "one" should receive ReadyForQuery "I" + And pg_doorman log does not contain "clearing prepared statement cache" + + Examples: + | reset_query | + | DISCARD ALL | + | RESET ALL; DEALLOCATE ALL; CLOSE ALL | diff --git a/tests/bdd/features/cleanup-policy-adaptive.feature b/tests/bdd/features/cleanup-policy-adaptive.feature new file mode 100644 index 000000000..3791de2ae --- /dev/null +++ b/tests/bdd/features/cleanup-policy-adaptive.feature @@ -0,0 +1,160 @@ +@rust @rust-3 @cleanup-policy-adaptive +Feature: Built-in cleanup without cleanup_server_query + A configured query belongs to `always`. Without one, `adaptive` sends built-in + cleanup only when it observed a session change, and `off` sends none. `off` still + rolls back a transaction the client left open. The mode is inherited by every pool. + + Background: + Given PostgreSQL started with options "-c log_statement=all -c logging_collector=off" and pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + + @cleanup-policy-adaptive-modes + Scenario Outline: Mode with pool setting sends built-in cleanups + Given pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 trust" + cleanup_server_connections = + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "session" + + + [[pools.example_db.users]] + username = "example_user_1" + password = "" + pool_size = 1 + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" + And we sleep 100ms + When we truncate PostgreSQL log + And we send SimpleQuery "" to session "one" + And we send SimpleQuery "" to session "one" + And we close session "one" + And we sleep 300ms + Then PostgreSQL log should contain exactly occurrences of "RESET ALL" + + Examples: + | mode | mode_override | first_query | second_query | resets | + | "adaptive" | # inherit | SELECT 1 | SELECT 1 | 0 | + | "adaptive" | # inherit | SET statement_timeout = 10000 | SELECT 1 | 1 | + | "adaptive" | # inherit | SET statement_timeout = 10000 | DISCARD ALL | 0 | + | "adaptive" | # inherit | BEGIN; SET statement_timeout = 10000 | SELECT 1 | 1 | + | "off" | # inherit | SET statement_timeout = 10000 | SELECT 1 | 0 | + | "adaptive" | cleanup_server_connections = "off" | SET statement_timeout = 10000 | SELECT 1 | 0 | + | "off" | cleanup_server_connections = "adaptive" | SET statement_timeout = 10000 | SELECT 1 | 1 | + | "adaptive" | cleanup_server_connections = true | SET statement_timeout = 10000 | SELECT 1 | 1 | + | "adaptive" | cleanup_server_connections = false | SET statement_timeout = 10000 | SELECT 1 | 0 | + + @cleanup-policy-adaptive-rollback-split + Scenario Outline: Mode rolls back an abandoned transaction and resets session state only in adaptive + Given pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 trust" + cleanup_server_connections = + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "session" + + [[pools.example_db.users]] + username = "example_user_1" + password = "" + pool_size = 1 + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" + And we sleep 100ms + When we truncate PostgreSQL log + And we send SimpleQuery "" to session "one" + And we close session "one" + And we sleep 300ms + # The transaction rollback belongs to every mode: a client that leaves a + # transaction open must not hand it to the next client. + Then PostgreSQL log should contain exactly occurrences of "ROLLBACK" + # The RESET sequence belongs to adaptive only. off never sends it, even when + # the session state changed. + And PostgreSQL log should contain exactly occurrences of "RESET ROLE" + And PostgreSQL log should contain exactly occurrences of "RESET ALL" + And PostgreSQL log should contain exactly occurrences of "CLOSE ALL" + # DEALLOCATE ALL belongs to the PREPARE flag alone: cursors and SET must + # not drag the pooler-side prepared statement cache down with them. + And PostgreSQL log should contain exactly occurrences of "DEALLOCATE ALL" + + Examples: + | mode | client_query | rollbacks | reset_role | reset_all | close_all | deallocates | + | "off" | BEGIN; SET statement_timeout = 10000 | 1 | 0 | 0 | 0 | 0 | + | "adaptive" | BEGIN; SET statement_timeout = 10000 | 1 | 1 | 1 | 0 | 0 | + | "off" | BEGIN; DECLARE doorman_cur CURSOR FOR SELECT 1 | 1 | 0 | 0 | 0 | 0 | + | "adaptive" | BEGIN; DECLARE doorman_cur CURSOR FOR SELECT 1 | 1 | 1 | 0 | 1 | 0 | + | "off" | SET statement_timeout = 10000 | 0 | 0 | 0 | 0 | 0 | + | "adaptive" | SET statement_timeout = 10000 | 0 | 1 | 1 | 0 | 0 | + | "off" | BEGIN; DECLARE doorman_cur CURSOR WITH HOLD FOR SELECT 1; COMMIT | 0 | 0 | 0 | 0 | 0 | + | "adaptive" | BEGIN; DECLARE doorman_cur CURSOR WITH HOLD FOR SELECT 1; COMMIT | 0 | 1 | 0 | 1 | 0 | + | "off" | SELECT 1 | 0 | 0 | 0 | 0 | 0 | + | "adaptive" | SELECT 1 | 0 | 0 | 0 | 0 | 0 | + | "off" | LISTEN split_ch | 0 | 0 | 0 | 0 | 0 | + | "adaptive" | LISTEN split_ch | 0 | 1 | 0 | 0 | 0 | + | "off" | PREPARE split_stmt AS SELECT 1 | 0 | 0 | 0 | 0 | 0 | + | "adaptive" | PREPARE split_stmt AS SELECT 1 | 0 | 1 | 0 | 0 | 1 | + | "off" | CREATE TABLE split_perm (i int) | 0 | 0 | 0 | 0 | 0 | + | "adaptive" | CREATE TABLE split_perm (i int) | 0 | 1 | 0 | 0 | 0 | + + @cleanup-policy-adaptive-prepared-desync + Scenario Outline: Mode re-synchronizes the prepared statement cache only in adaptive + Given pg_doorman started with config: + """ + [general] + host = "127.0.0.1" + port = ${DOORMAN_PORT} + admin_username = "admin" + admin_password = "admin" + pg_hba.content = "host all all 127.0.0.1/32 trust" + prepared_statements = true + cleanup_server_connections = + + [pools.example_db] + server_host = "127.0.0.1" + server_port = ${PG_PORT} + pool_mode = "session" + + [[pools.example_db.users]] + username = "example_user_1" + password = "" + pool_size = 1 + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" + And we sleep 100ms + When we truncate PostgreSQL log + # pg_doorman keeps the Parse in its own buffer until Sync, so PostgreSQL never + # sees this statement. The client disconnects before Sync. + And we send Parse "desync" with query "SELECT 42" to session "one" + And we send Bind "" to "desync" with params "" to session "one" + And we send Execute "" to session "one" + And we close session "one" + And we sleep 300ms + Then PostgreSQL log should contain exactly occurrences of "DEALLOCATE ALL" + And PostgreSQL log should contain exactly occurrences of "RESET ROLE" + + Examples: + | mode | deallocates | reset_role | + | "off" | 0 | 0 | + | "adaptive" | 1 | 1 | diff --git a/tests/bdd/features/cleanup-policy.feature b/tests/bdd/features/cleanup-policy.feature new file mode 100644 index 000000000..2a4ec40c6 --- /dev/null +++ b/tests/bdd/features/cleanup-policy.feature @@ -0,0 +1,514 @@ +@rust @rust-3 @cleanup-server-query +Feature: Backend cleanup policy + + Background: + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + + @cleanup-server-query-session-reuse + Scenario Outline: Reuse only after a successful full reset + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + cleanup_server_connections: always + cleanup_server_query: "" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "SET statement_timeout = 10000; CREATE TEMP TABLE reset_secret (value int); PREPARE reset_stmt AS SELECT 42" to session "old" + And we send SimpleQuery "SELECT current_setting('statement_timeout') = '10s' AND to_regclass('pg_temp.reset_secret') IS NOT NULL AND EXISTS (SELECT FROM pg_prepared_statements WHERE name = 'reset_stmt')" to session "old" and store response + Then session "old" should receive DataRow with "t" + When we send SimpleQuery "BEGIN" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should backend_pid from session "old" + When we send SimpleQuery "SELECT current_setting('statement_timeout') = '0' AND to_regclass('pg_temp.reset_secret') IS NULL AND NOT EXISTS (SELECT FROM pg_prepared_statements WHERE name = 'reset_stmt')" to session "next" and store response + Then session "next" should receive DataRow with "t" + And session "next" should receive ReadyForQuery "I" + + Examples: + | reset_query | comparison | + | RESET ALL; DEALLOCATE ALL; CLOSE ALL; UNLISTEN *; SELECT repeat('x', 20000), pg_advisory_unlock_all(); DISCARD PLANS; DISCARD SEQUENCES; DISCARD TEMP; SET SESSION AUTHORIZATION DEFAULT | equal | + | RESET ALL; DEALLOCATE ALL; SELECT 1 / 0; DISCARD TEMP | not equal | + | RESET ALL; DEALLOCATE ALL; DISCARD TEMP; DO $$ BEGIN RAISE NOTICE USING ERRCODE = '0AM01', MESSAGE = 'cleanup notice'; END $$ | equal | + | RESET ALL; DEALLOCATE ALL; DISCARD TEMP; DO $$ BEGIN RAISE NOTICE USING ERRCODE = '0A000', MESSAGE = 'cleanup notice'; END $$ | equal | + | BEGIN | not equal | + | COPY reset_secret FROM STDIN | not equal | + | SELECT pg_sleep(5) | not equal | + | /* empty reset */ | not equal | + + @cleanup-server-query-prepared-reuse + Scenario: Cached Bind reparses and restores startup parameters after reset + Given fixtures from "tests/fixture-search-path.sql" applied + And pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + prepared_statements: true + sync_server_parameters: true + cleanup_server_connections: always + cleanup_server_query: "SELECT 1 / 0" + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + cleanup_server_query: "DISCARD ALL" + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" and startup parameters "search_path=schema_a" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid as "before" + And we send Parse "cached" with query "SELECT val + $1::int FROM t" to session "one" + And we send Bind "" to "cached" with params "10" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "11" + And session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements" to session "one" and store response + # `always` already ran the pool-level DISCARD ALL on the previous Sync. + Then session "one" should receive DataRow with "0" + When we send SimpleQuery "SET statement_timeout = 10000" to session "one" + And we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements" to session "one" and store response + Then session "one" should receive DataRow with "0" + When we send Bind "" to "cached" with params "11" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "12" + And session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid as "after" + Then named backend_pid "after" from session "one" is same as "before" + + @cleanup-server-query-policy + Scenario Outline: Cleanup scheduling counts each released backend once + When I run shell command: + """ + psql -X -h 127.0.0.1 -p ${PG_PORT} -U postgres -d example_db -v ON_ERROR_STOP=1 -c "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" + """ + Then the command should succeed + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + server_idle_check_timeout: 1 + pooler_check_query: "SELECT 7" + cleanup_server_connections: + cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + " + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + + + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + When I run shell command: + """ + psql -X -h 127.0.0.1 -p ${PG_PORT} -U postgres -d example_db -v ON_ERROR_STOP=1 -c "UPDATE public.cleanup_counter SET n = 0" + """ + Then the command should succeed + When we send SimpleQuery "" to session "old" + When I run shell command: + """ + psql -X -h 127.0.0.1 -p ${PG_PORT} -U postgres -d example_db -At -v ON_ERROR_STOP=1 -c "SELECT n FROM public.cleanup_counter" + """ + Then the command should succeed + And the command output should contain "" + When we sleep 10ms + And we send SimpleQuery "" to session "old" and store response + Then session "old" should receive DataRow with "" + When we close session "old" + And we sleep 10ms + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + When we send SimpleQuery "SELECT count(*) = 1 AND min(n) = AND current_setting('statement_timeout') = '0' FROM public.cleanup_counter" to session "next" and store response + Then session "next" should receive DataRow with "t" + And session "next" should receive ReadyForQuery "T" + + Examples: + | mode | mode_override | increment | query_override | first_query | second_query | intermediate | resets | + | always | # inherit | 1 | # inherit | SELECT pg_backend_pid() | SELECT min(n) FROM public.cleanup_counter | 1 | 2 | + | always | # inherit | 1 | # inherit | SELECT 7 | SELECT min(n) FROM public.cleanup_counter | 1 | 2 | + | always | # inherit | 1 | # inherit | BEGIN; SELECT pg_backend_pid() | SELECT min(n) FROM public.cleanup_counter; COMMIT | 0 | 1 | + | off | # inherit | 1 | # inherit | BEGIN; INSERT INTO public.cleanup_counter VALUES (99); SELECT pg_backend_pid() | SELECT min(n) FROM public.cleanup_counter | 0 | 0 | + | off | cleanup_server_connections: always | 1 | # inherit | SELECT pg_backend_pid() | SELECT min(n) FROM public.cleanup_counter | 1 | 2 | + | always | cleanup_server_connections: off | 1 | # inherit | BEGIN; INSERT INTO public.cleanup_counter VALUES (99); SELECT pg_backend_pid() | SELECT min(n) FROM public.cleanup_counter | 0 | 0 | + | always | cleanup_server_connections: false | 1 | # inherit | BEGIN; INSERT INTO public.cleanup_counter VALUES (99); SELECT pg_backend_pid() | SELECT min(n) FROM public.cleanup_counter | 0 | 0 | + | always | # inherit | 100 | cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + 1" | SELECT pg_backend_pid() | SELECT min(n) FROM public.cleanup_counter | 1 | 2 | + + @cleanup-async @cleanup-async-cache + Scenario Outline: Flush and Sync honor pooling boundaries while Bind remains reusable + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + cleanup_server_connections: + + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + When we send Parse "probe" with query "SELECT pg_backend_pid(), current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "one" + And we send Bind "" to "probe" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12DC" from session "one" + And we store backend_pid from last response of session "one" + Then session "one" should receive exactly one DataRow "${one_pid}|0|1" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we send Sync to session "one" + Then session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "" + When we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "after" + Then backend_pid "after" from session "one" should equal initial backend_pid from session "one" + When we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements" to session "one" and store response + Then session "one" should receive exactly one DataRow "" + When we send Bind "" to "probe" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "2DC" from session "one" + Then session "one" should receive exactly one DataRow "${one_pid}|0|1" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "" + When we send Sync to session "one" + Then session "one" should receive ReadyForQuery "T" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "" + When we send SimpleQuery "ROLLBACK" to session "one" and store response + Then session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "" + + When we close session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + When we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements" to session "next" and store response + Then session "next" should receive exactly one DataRow "" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "" + + Examples: + | pool_mode | mode | query_config | resets | cached | total | after_close_resets | after_close_cached | + | transaction | adaptive | # inherit | 0 | 1 | 0 | 0 | 1 | + | transaction | always | cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + 1" | 1 | 0 | 2 | 2 | 0 | + | session | always | cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + 1" | 0 | 1 | 0 | 1 | 0 | + + @cleanup-async @cleanup-async-set + Scenario Outline: Extended SET remains visible until Sync releases the backend + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + cleanup_server_connections: + + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + When we send Parse "set_timeout" with query "SET statement_timeout = 10000" to session "one" + And we send Bind "" to "set_timeout" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12C" from session "one" + When we send Parse "probe" with query "SELECT pg_backend_pid(), current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "one" + And we send Bind "" to "probe" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12DC" from session "one" + And we store backend_pid from last response of session "one" + Then session "one" should receive exactly one DataRow "${one_pid}|10s|2" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we send Sync to session "one" + Then session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "" + When we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + When we send SimpleQuery "SELECT current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "next" and store response + Then session "next" should receive exactly one DataRow "|" + And session "next" should receive ReadyForQuery "T" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "" + + Examples: + | mode | query_config | resets | timeout | cached | + | adaptive | # inherit | 0 | 0 | 2 | + | always | cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + 1" | 1 | 0 | 0 | + | off | # inherit | 0 | 10s | 2 | + + @cleanup-async @cleanup-async-transaction + Scenario Outline: Sync inside a transaction defers always cleanup until transaction end + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + cleanup_server_connections: always + cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + 1" + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "before" + When we send Parse "set_timeout" with query "SET statement_timeout = 10000" to session "one" + And we send Bind "" to "set_timeout" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12C" from session "one" + When we send Sync to session "one" + Then session "one" should receive ReadyForQuery "T" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we send Parse "probe" with query "SELECT pg_backend_pid(), current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "one" + And we send Bind "" to "probe" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12DC" from session "one" + And we store backend_pid from last response of session "one" + Then backend_pid "before" from session "one" should equal initial backend_pid from session "one" + And session "one" should receive exactly one DataRow "${one_pid}|10s|2" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we send Sync to session "one" + Then session "one" should receive ReadyForQuery "T" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we send SimpleQuery "" to session "one" and store response + Then session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "1" + When we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + When we send SimpleQuery "SELECT current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "next" and store response + Then session "next" should receive exactly one DataRow "0|0" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "1" + + Examples: + | end_transaction | + | COMMIT | + | ROLLBACK | + + @cleanup-async @cleanup-async-error + Scenario Outline: Parse error received before Sync preserves frontend recovery + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + cleanup_server_connections: + + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "" to session "one" and store backend_pid as "before" + When we send Parse "set_timeout" with query "SET statement_timeout = 10000" to session "one" + And we send Bind "" to "set_timeout" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12C" from session "one" + When we send Parse "" with query "SELECT FROM WHERE" to session "one" + And we send Flush to session "one" + And we read backend messages "E" from session "one" + Then session "one" should receive error containing "syntax error" with code "42601" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we send Sync to session "one" + Then session "one" should receive ReadyForQuery "" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we send SimpleQuery "" to session "one" and store response + Then session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "after" + Then named backend_pid "after" from session "one" is different from "before" + When we send SimpleQuery "SELECT current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "one" and store response + Then session "one" should receive exactly one DataRow "0|0" + And session "one" should receive ReadyForQuery "T" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + + Examples: + | mode | query_config | begin_query | status | finish_query | + | adaptive | # inherit | SELECT pg_backend_pid() | I | SELECT 1 | + | always | cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + 1" | BEGIN; SELECT pg_backend_pid() | E | ROLLBACK | + + @cleanup-async @cleanup-async-disconnect + Scenario: Disconnect after a drained Flush cleans the backend before reuse + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + cleanup_server_connections: always + cleanup_server_query: "RESET ALL; DEALLOCATE ALL; CLOSE ALL; UPDATE public.cleanup_counter SET n = n + 1" + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + When we send Parse "set_timeout" with query "SET statement_timeout = 10000" to session "one" + And we send Bind "" to "set_timeout" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12C" from session "one" + When we send Parse "probe" with query "SELECT pg_backend_pid(), current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "one" + And we send Bind "" to "probe" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12DC" from session "one" + And we store backend_pid from last response of session "one" + Then session "one" should receive exactly one DataRow "${one_pid}|10s|2" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "0" + When we abort TCP connection with RST for session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "next" without waiting + Then we read SimpleQuery response from session "next" within 5000ms + When we store backend_pid from last response of session "next" + Then backend_pid from session "next" should equal backend_pid from session "one" + When we send SimpleQuery "SELECT current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "next" and store response + Then session "next" should receive exactly one DataRow "0|0" + And session "next" should receive ReadyForQuery "T" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "1" + + @cleanup-async @cleanup-async-reset-error @cleanup-review + Scenario: Failed cleanup after Flush preserves ReadyForQuery and retires the dirty backend + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + cleanup_server_connections: always + cleanup_server_query: "SELECT nextval('public.cleanup_attempts'); RESET ALL; DEALLOCATE ALL; SELECT 1 / 0" + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "CREATE SEQUENCE public.cleanup_attempts" to session "observer" + When we send Parse "set_timeout" with query "SET statement_timeout = 10000" to session "one" + And we send Bind "" to "set_timeout" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12C" from session "one" + When we send Parse "probe" with query "SELECT pg_backend_pid(), current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "one" + And we send Bind "" to "probe" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12DC" from session "one" + And we store backend_pid from last response of session "one" as "before" + Then session "one" should receive exactly one DataRow "${one_pid}|10s|2" + When we send SimpleQuery "SELECT is_called FROM public.cleanup_attempts" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "f" + When we send Sync to session "one" + Then session "one" should receive ReadyForQuery "I" + And we send SimpleQuery "SELECT last_value, is_called FROM public.cleanup_attempts" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "1|t" + When we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "after" + Then named backend_pid "after" from session "one" is different from "before" + When we send SimpleQuery "SELECT current_setting('statement_timeout'), (SELECT count(*) FROM pg_prepared_statements)" to session "one" and store response + Then session "one" should receive exactly one DataRow "0|0" + And session "one" should receive ReadyForQuery "T" diff --git a/tests/bdd/features/cleanup-prepare.feature b/tests/bdd/features/cleanup-prepare.feature new file mode 100644 index 000000000..6edd636d2 --- /dev/null +++ b/tests/bdd/features/cleanup-prepare.feature @@ -0,0 +1,219 @@ +@rust @rust-3 @cleanup-review @cleanup-prepare +Feature: Adaptive cleanup removes SQL PREPARE statements left by a client + SQL PREPARE and an extended-protocol Parse share one server-side + statement namespace. An untracked PREPARE made the next client on the + same backend fail with 42P05 duplicate_prepared_statement. + The checkin DEALLOCATE ALL also removes every pooler-cached DOORMAN_* + statement of that backend. A broken cache drop surfaces on the next + client as 26000 prepared statement does not exist on a cached Bind. + The scenarios cover single statements, pipelined batches, multi-statement + Query batches, and the two error paths that arm the same flag. + + Background: + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + And pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + web: + enabled: true + host: "127.0.0.1" + port: 9129 + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + example_db_session: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + server_database: "example_db" + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + + @cleanup-prepare-statement + Scenario: A SQL PREPARE statement does not survive a checkin + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "PREPARE leak_stmt AS SELECT 42" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + # The same name must be preparable again: 42P05 means the statement leaked. + And we send SimpleQuery "PREPARE leak_stmt AS SELECT 42" to session "next" and store response + Then session "next" should receive CommandComplete "PREPARE" + + @cleanup-prepare-deallocate-all + Scenario: A client DEALLOCATE ALL suppresses the checkin DEALLOCATE ALL + # Session pooling has a single checkin at disconnect, so PREPARE and the + # client's DEALLOCATE ALL share one lease. + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "PREPARE keep_stmt AS SELECT 42" to session "old" + And we send SimpleQuery "DEALLOCATE ALL" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements" to session "next" and store response + Then session "next" should receive DataRow with "0" + # The client's own DEALLOCATE ALL already emptied the view, so the counter + # is what pins suppression. Its label is the pool name, and creating the + # next lease waits for the previous checkin to finish. + When I run shell command: + """ + python3 - <<'PY' + import time + import urllib.request + deadline = time.monotonic() + 5 + while True: + body = urllib.request.urlopen('http://127.0.0.1:9129/metrics', timeout=2).read().decode() + lines = body.splitlines() + ok = 'pg_doorman_server_cleanup_total{database="example_db_session",result="ok",user="example_user_1"} 0' in lines + err = 'pg_doorman_server_cleanup_total{database="example_db_session",result="error",user="example_user_1"} 0' in lines + if ok and err: + break + assert time.monotonic() < deadline, body + time.sleep(0.05) + PY + """ + Then the command should succeed + + @cleanup-prepare-mixed-cache + Scenario: A SQL PREPARE checkin invalidates the cached extended statement + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid + And we send Parse "mixed" with query "select $1::int + $1::int" to session "one" + And we send Bind "" to "mixed" with params "20" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "40" + # The SQL PREPARE tag arms the flag; the next checkin sends DEALLOCATE ALL, + # which removes both the manual statement and the cached DOORMAN_* name. + When we send SimpleQuery "PREPARE mixed_manual AS SELECT 1" to session "one" and store response + Then session "one" should receive CommandComplete "PREPARE" + When we close session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + # If the checkin left the stale LRU entry behind, this Parse would be + # skipped as a cache hit and the Bind would fail with 26000. + And we send Parse "mixed" with query "select $1::int + $1::int" to session "next" + And we send Bind "" to "mixed" with params "5" to session "next" + And we send Execute "" to session "next" + And we send Sync to session "next" + Then session "next" should receive DataRow with "10" + When we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements WHERE name = 'mixed_manual'" to session "next" and store response + Then session "next" should receive DataRow with "0" + + @cleanup-prepare-batch + Scenario: A pipelined batch and a multi-statement Query batch invalidate the cache together + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid + # One pipelined extended batch: two Parses, two Binds, two Executes, one Sync. + And we send Parse "batch_a" with query "select $1::int + 1" to session "one" + And we send Parse "batch_b" with query "select $1::int * 2" to session "one" + And we send Bind "" to "batch_a" with params "10" to session "one" + And we send Execute "" to session "one" + And we send Bind "" to "batch_b" with params "10" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "11" + # A multi-statement Query: the PREPARE tag arms the flag mid-batch. + When we send SimpleQuery "SELECT 1; PREPARE batch_manual AS SELECT 2; SELECT 3" to session "one" and store response + Then session "one" should receive CommandComplete "PREPARE" + When we close session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + # Both cached entries must be gone: each Bind re-parses instead of + # failing with 26000. + And we send Parse "batch_a" with query "select $1::int + 1" to session "next" + And we send Bind "" to "batch_a" with params "100" to session "next" + And we send Execute "" to session "next" + And we send Sync to session "next" + Then session "next" should receive DataRow with "101" + And we send Parse "batch_b" with query "select $1::int * 2" to session "next" + And we send Bind "" to "batch_b" with params "100" to session "next" + And we send Execute "" to session "next" + And we send Sync to session "next" + Then session "next" should receive DataRow with "200" + + @cleanup-prepare-query-error + Scenario: A failed query checkin invalidates the cached extended statement + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid + And we send Parse "err_cached" with query "select $1::int * 3" to session "one" + And we send Bind "" to "err_cached" with params "14" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "42" + # Any ErrorResponse arms prepare-cleanup while the cache is enabled. + When we send SimpleQuery "SELECT 1 / 0" to session "one" expecting error + Then session "one" should receive ErrorResponse with SQLSTATE "22012" + When we close session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + And we send Parse "err_cached" with query "select $1::int * 3" to session "next" + And we send Bind "" to "err_cached" with params "7" to session "next" + And we send Execute "" to session "next" + And we send Sync to session "next" + Then session "next" should receive DataRow with "21" + + @cleanup-prepare-parse-error + Scenario: A failed Parse in a batch still invalidates the earlier cached statement + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid + And we send Parse "good_early" with query "select $1::int + 100" to session "one" + And we send Bind "" to "good_early" with params "1" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "101" + # The broken Parse fails at Sync. Its registration rolls back, but the + # error still arms prepare-cleanup for the surviving cached entry. + When we send Parse "broken" with query "selec broken syntax here" to session "one" + And we send Sync to session "one" + Then session "one" should receive ErrorResponse with SQLSTATE "42601" + When we close session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + And we send Parse "good_early" with query "select $1::int + 100" to session "next" + And we send Bind "" to "good_early" with params "23" to session "next" + And we send Execute "" to session "next" + And we send Sync to session "next" + Then session "next" should receive DataRow with "123" + + @cleanup-prepare-session-named-parse + Scenario: A session-mode named Parse does not leak to the next client + # Session pooling forwards Parse as-is: the client owns the backend for + # the whole connection. The forwarded name must not outlive the lease. + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid + And we send Parse "forwarded" with query "select $1::int + 5" to session "one" + And we send Bind "" to "forwarded" with params "1" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive DataRow with "6" + When we close session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "one" + # 42P05 here would mean the forwarded statement leaked across clients. + And we send Parse "forwarded" with query "select $1::int + 5" to session "next" + And we send Bind "" to "forwarded" with params "2" to session "next" + And we send Execute "" to session "next" + And we send Sync to session "next" + Then session "next" should receive DataRow with "7" diff --git a/tests/bdd/features/cleanup-response-review.feature b/tests/bdd/features/cleanup-response-review.feature new file mode 100644 index 000000000..893bdfea9 --- /dev/null +++ b/tests/bdd/features/cleanup-response-review.feature @@ -0,0 +1,124 @@ +@rust @rust-3 @cleanup-review @cleanup-review-response +Feature: Cleanup preserves completed client responses + + Background: + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.committed_rows (id int PRIMARY KEY)" to session "observer" + + @cleanup-review-commit + Scenario Outline: Cleanup errors and notices preserve COMMIT responses + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + cleanup_server_connections: always + cleanup_server_query: "" + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "before" + And we send SimpleQuery "INSERT INTO public.committed_rows VALUES (1)" to session "one" + And we send SimpleQuery "COMMIT" to session "one" and store response + Then session "one" should receive CommandComplete "COMMIT" + And session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT count(*) FROM public.committed_rows" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "1" + When we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "after" + Then named backend_pid "after" from session "one" is "before" + When we send SimpleQuery "SELECT count(*) FROM public.committed_rows" to session "one" and store response + Then session "one" should receive exactly one DataRow "1" + When we send SimpleQuery "ROLLBACK" to session "one" and store response + Then session "one" should receive CommandComplete "ROLLBACK" + And session "one" should receive ReadyForQuery "I" + + Examples: + | reset_query | comparison | + | SELECT 1 / 0 | different from | + | DO $$ BEGIN RAISE NOTICE USING ERRCODE = '0AM01', MESSAGE = 'cleanup notice'; END $$ | same as | + | DO $$ BEGIN RAISE NOTICE USING ERRCODE = '0A000', MESSAGE = 'cleanup notice'; END $$ | same as | + | BEGIN | different from | + | COPY public.committed_rows FROM STDIN | different from | + | SELECT pg_sleep(5) | different from | + | /* empty reset */ | different from | + + @cleanup-review-legacy-flush + Scenario: Built-in cleanup drains its own response after a disconnected Flush + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send Parse "set_timeout" with query "SET statement_timeout = 10000" to session "one" + And we send Bind "" to "set_timeout" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12C" from session "one" + And we send Parse "probe" with query "SELECT pg_backend_pid(), current_setting('statement_timeout')" to session "one" + And we send Bind "" to "probe" with params "" to session "one" + And we send Execute "" to session "one" + And we send Flush to session "one" + And we read backend messages "12DC" from session "one" + And we store backend_pid from last response of session "one" + Then session "one" should receive exactly one DataRow "${one_pid}|10s" + When we abort TCP connection with RST for session "one" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid(), current_setting('statement_timeout')" to session "next" without waiting + Then we read SimpleQuery response from session "next" within 5000ms + And session "next" should receive exactly one DataRow "${one_pid}|0" + And session "next" should receive ReadyForQuery "T" + + @cleanup-review-check-query + Scenario: A failed reset after a pooler check keeps the frontend usable + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pooler_check_query: "SELECT 7" + cleanup_server_connections: always + cleanup_server_query: "SELECT 1 / 0" + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT 7" to session "one" and store response + Then session "one" should receive exactly one DataRow "7" + And session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT 8" to session "one" and store response + Then session "one" should receive exactly one DataRow "8" + And session "one" should receive ReadyForQuery "I" diff --git a/tests/bdd/features/cleanup-state-review.feature b/tests/bdd/features/cleanup-state-review.feature new file mode 100644 index 000000000..d011a3771 --- /dev/null +++ b/tests/bdd/features/cleanup-state-review.feature @@ -0,0 +1,254 @@ +@rust @rust-3 @cleanup-review @cleanup-state-review +Feature: Cleanup preserves isolation and restores client startup parameters + + Background: + Given PostgreSQL started with options "-c log_statement=all -c logging_collector=off" and pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + + @cleanup-review-startup-reset + Scenario: Built-in RESET ALL restores untracked startup parameters on the same backend + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + sync_server_parameters: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" and startup parameters "work_mem=96MB" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SHOW work_mem" to session "first" and store response + Then session "first" should receive DataRow with "96MB" + When we send SimpleQuery "SET statement_timeout = 10000" to session "first" + And we send SimpleQuery "SHOW work_mem" to session "first" and store response + Then session "first" should receive DataRow with "96MB" + When we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" and startup parameters "work_mem=96MB" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SHOW work_mem" to session "next" and store response + Then session "next" should receive DataRow with "96MB" + + @cleanup-review-cursor-startup + Scenario: Cursor-only cleanup restores role and remembers unchanged startup parameters + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + sync_server_parameters: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" and startup parameters "role=example_user_2,work_mem=96MB" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SELECT current_user = 'example_user_2' AND current_setting('work_mem') = '96MB'" to session "first" and store response + Then session "first" should receive DataRow with "t" + When we send SimpleQuery "BEGIN; DECLARE review_cursor CURSOR WITH HOLD FOR SELECT 1; COMMIT" to session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" and startup parameters "role=example_user_2" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT current_user" to session "next" and store response + Then session "next" should receive DataRow with "example_user_2" + When we send SimpleQuery "SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem'" to session "next" and store response + Then session "next" should receive DataRow with "t" + When we send SimpleQuery "SELECT count(*) FROM pg_cursors" to session "next" and store response + Then session "next" should receive DataRow with "0" + + @cleanup-review-single-reset + Scenario Outline: A partial client RESET cancels only built-in parameter cleanup + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + + + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SET work_mem = '96MB'; SET statement_timeout = 10000; RESET statement_timeout" to session "first" + And we send SimpleQuery "SELECT current_setting('work_mem') = '96MB' AND current_setting('statement_timeout') = '0'" to session "first" and store response + Then session "first" should receive DataRow with "t" + When we close session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem'" to session "next" and store response + Then session "next" should receive DataRow with "" + + Examples: + | mode_config | query_config | is_default | + | # adaptive | # built-in cleanup | f | + | cleanup_server_connections: always | cleanup_server_query: "RESET ALL" | t | + + @cleanup-review-client-reset-commands + Scenario Outline: Client reset commands clear only their own built-in cleanup obligation + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we sleep 100ms + When we truncate PostgreSQL log + And we send SimpleQuery "" to session "first" + And we send SimpleQuery "" to session "first" + And we close session "first" + And we sleep 300ms + Then PostgreSQL log should contain exactly occurrences of "RESET ROLE" + When we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT (SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem') AND NOT EXISTS (SELECT FROM pg_cursors) AND NOT EXISTS (SELECT FROM pg_prepared_statements)" to session "next" and store response + Then session "next" should receive DataRow with "t" + + Examples: + # Built-in cleanup starts with RESET ROLE, so the log counts it and no client + # command in these rows produces that tag. + | setup | client_reset | cleanups | + | SET work_mem = '96MB' | DISCARD ALL | 0 | + | PREPARE review_stmt AS SELECT 1 | DEALLOCATE ALL | 0 | + | SET work_mem = '96MB'; PREPARE review_stmt AS SELECT 1 | DEALLOCATE ALL | 1 | + | BEGIN; DECLARE review_cursor CURSOR WITH HOLD FOR SELECT 1; COMMIT | CLOSE ALL | 0 | + | SET work_mem = '96MB'; BEGIN; DECLARE review_cursor CURSOR WITH HOLD FOR SELECT 1; COMMIT | CLOSE ALL | 1 | + + @cleanup-review-deallocate-after-error + Scenario: Client DEALLOCATE ALL clears the prepared cleanup obligation after an error + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we sleep 100ms + When we truncate PostgreSQL log + And we send SimpleQuery "PREPARE review_stmt AS SELECT 1" to session "first" + And we send SimpleQuery "SELECT 1 / 0" to session "first" expecting error + And we send SimpleQuery "DEALLOCATE ALL" to session "first" + And we close session "first" + And we sleep 300ms + Then PostgreSQL log should contain exactly 0 occurrences of "RESET ROLE" + When we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements" to session "next" and store response + Then session "next" should receive DataRow with "0" + + @cleanup-review-reload + Scenario Outline: RELOAD changes cleanup policy for new frontends while existing frontends retain their pool + When we create session "observer" to postgres as "postgres" with password "" and database "example_db" + And we send SimpleQuery "CREATE TABLE public.cleanup_counter (n int); INSERT INTO public.cleanup_counter VALUES (0)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + cleanup_server_connections: always + cleanup_server_query: "RESET ALL; UPDATE public.cleanup_counter SET n = n + 1" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid as "before_reload" + And we send SimpleQuery "SET work_mem = '96MB'" to session "old" + And we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive DataRow with "3" + When we overwrite pg_doorman config file with: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + cleanup_server_connections: + + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + And we create admin session "admin" to pg_doorman as "admin" with password "admin" + And we execute "RELOAD" on admin session "admin" + And we send SimpleQuery "SET work_mem = '96MB'" to session "old" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid as "after_reload" + Then named backend_pid "after_reload" from session "old" is same as "before_reload" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive DataRow with "5" + When we create session "new" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SET work_mem = '96MB'; SELECT pg_backend_pid()" to session "new" and store backend_pid + Then backend_pid from session "new" should not equal backend_pid from session "old" + When we send SimpleQuery "SELECT n FROM public.cleanup_counter" to session "observer" and store response + Then session "observer" should receive DataRow with "" + When we send SimpleQuery "SELECT current_setting('work_mem') = '96MB'" to session "new" and store response + Then session "new" should receive DataRow with "" + + Examples: + # The `old` frontend keeps the backend it was created with, so the two queries + # it sends after RELOAD are cleaned by the policy of the first config. Every + # row changes the policy, so `new` frontends get a backend of a rebuilt pool. + | mode | query_config | total | work_mem_kept | + | always | cleanup_server_query: "RESET ALL; UPDATE public.cleanup_counter SET n = n + 2" | 7 | f | + | adaptive | # built-in cleanup | 5 | f | + | off | cleanup_server_query: "RESET ALL; UPDATE public.cleanup_counter SET n = n + 1" | 5 | t | diff --git a/tests/bdd/features/cleanup-temp-tables.feature b/tests/bdd/features/cleanup-temp-tables.feature new file mode 100644 index 000000000..e91b19cab --- /dev/null +++ b/tests/bdd/features/cleanup-temp-tables.feature @@ -0,0 +1,258 @@ +@rust @rust-3 @cleanup-review @cleanup-temp-tables +Feature: Adaptive cleanup removes temporary tables left by a client + The built-in cleanup tracks the CREATE TABLE command tag and sends + DISCARD TEMP at checkin. `SELECT ... INTO TEMP` and `CREATE TEMP TABLE + AS SELECT` complete with the inner query's tag on current PostgreSQL + and are not tracked. Permanent objects created by CREATE TABLE are not + dropped: DISCARD TEMP only touches temporary objects. + + Background: + Given PostgreSQL started with pg_hba.conf: + """ + local all all trust + host all all 127.0.0.1/32 trust + """ + And fixtures from "tests/fixture.sql" applied + + @cleanup-temp-tables-transaction + Scenario: A temp table does not survive a checkin in transaction mode + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "CREATE TEMP TABLE leak_a (i int)" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + And we send SimpleQuery "SELECT to_regclass('pg_temp.leak_a') IS NULL" to session "next" and store response + Then session "next" should receive DataRow with "t" + And session "next" should receive ReadyForQuery "I" + + @cleanup-temp-tables-session + Scenario: A temp table does not survive a checkin in session mode + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: session + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "CREATE TEMP TABLE leak_session (i int)" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + And we send SimpleQuery "SELECT to_regclass('pg_temp.leak_session') IS NULL" to session "next" and store response + Then session "next" should receive DataRow with "t" + + @cleanup-temp-tables-permanent + Scenario: A permanent table survives the temp-cleanup checkin + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + web: + enabled: true + host: "127.0.0.1" + port: 9129 + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + And we send SimpleQuery "CREATE TABLE leak_perm (i int)" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + # The CREATE TABLE tag arms the temp cleanup; DISCARD TEMP must not touch + # the permanent table while the checkin itself becomes observable. + And we send SimpleQuery "SELECT to_regclass('public.leak_perm') IS NOT NULL" to session "next" and store response + Then session "next" should receive DataRow with "t" + When I run shell command: + """ + python3 - <<'PY' + import time + import urllib.request + deadline = time.monotonic() + 5 + while True: + body = urllib.request.urlopen('http://127.0.0.1:9129/metrics', timeout=2).read().decode() + lines = body.splitlines() + if 'pg_doorman_server_cleanup_total{database="example_db",result="ok",user="example_user_1"} 1' in lines: + break + assert time.monotonic() < deadline, body + time.sleep(0.05) + PY + """ + Then the command should succeed + + @cleanup-temp-tables-readonly + Scenario: Read-only clients do not trigger any cleanup + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + web: + enabled: true + host: "127.0.0.1" + port: 9129 + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT 1" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT 1" to session "next" + And I run shell command: + """ + python3 - <<'PY' + import time + import urllib.request + deadline = time.monotonic() + 5 + while True: + body = urllib.request.urlopen('http://127.0.0.1:9129/metrics', timeout=2).read().decode() + lines = body.splitlines() + ok = 'pg_doorman_server_cleanup_total{database="example_db",result="ok",user="example_user_1"} 0' in lines + err = 'pg_doorman_server_cleanup_total{database="example_db",result="error",user="example_user_1"} 0' in lines + if ok and err: + break + assert time.monotonic() < deadline, body + time.sleep(0.05) + PY + """ + Then the command should succeed + + @cleanup-temp-tables-advisory + Scenario: The cleanup batch releases session-level advisory locks + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + # The SET arms the built-in cleanup batch; the advisory lock has no + # command tag and rides on the batch via pg_advisory_unlock_all(). + And we send SimpleQuery "SELECT pg_advisory_lock(424242)" to session "old" + And we send SimpleQuery "SET statement_timeout = 10000" to session "old" + And we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + And we send SimpleQuery "SELECT count(*) FROM pg_locks WHERE locktype = 'advisory'" to session "next" and store response + Then session "next" should receive DataRow with "0" + + @cleanup-temp-tables-advisory-lone + Scenario: A lone advisory lock survives the checkin + # The lock has no command tag: it does not arm the cleanup batch, and no + # other statement arms it either. No batch runs, and the lock reaches the + # next client on the same backend. This is the documented hole in + # "Not tracked": release such locks in the application, or use `always`. + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + connect_timeout: 1000 + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + web: + enabled: true + host: "127.0.0.1" + port: 9129 + pools: + example_db: + server_host: "127.0.0.1" + server_port: ${PG_PORT} + pool_mode: transaction + users: [{username: example_user_1, password: "", pool_size: 1}] + """ + When we create session "old" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "old" and store backend_pid + # pg_advisory_lock answers with the plain SELECT tag: nothing is armed. + And we send SimpleQuery "SELECT pg_advisory_lock(10, 20)" to session "old" + And we send SimpleQuery "SELECT count(*) FROM pg_locks WHERE locktype = 'advisory' AND classid = 10 AND objid = 20" to session "old" and store response + Then session "old" should receive DataRow with "1" + When we close session "old" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "old" + # The lock outlived the client that took it. + And we send SimpleQuery "SELECT count(*) FROM pg_locks WHERE locktype = 'advisory' AND classid = 10 AND objid = 20" to session "next" and store response + Then session "next" should receive DataRow with "1" + # No cleanup batch ran: both counters stayed at zero. + When I run shell command: + """ + python3 - <<'PY' + import time + import urllib.request + deadline = time.monotonic() + 5 + while True: + body = urllib.request.urlopen('http://127.0.0.1:9129/metrics', timeout=2).read().decode() + lines = body.splitlines() + ok = 'pg_doorman_server_cleanup_total{database="example_db",result="ok",user="example_user_1"} 0' in lines + err = 'pg_doorman_server_cleanup_total{database="example_db",result="error",user="example_user_1"} 0' in lines + if ok and err: + break + assert time.monotonic() < deadline, body + time.sleep(0.05) + PY + """ + Then the command should succeed diff --git a/tests/bdd/features/client-session-reset-cleanup.feature b/tests/bdd/features/client-session-reset-cleanup.feature index 14331729e..5df244c90 100644 --- a/tests/bdd/features/client-session-reset-cleanup.feature +++ b/tests/bdd/features/client-session-reset-cleanup.feature @@ -1,28 +1,8 @@ @rust @rust-3 @client-session-reset-cleanup -Feature: Client session reset batch suppresses doorman-side cleanup - When a client (e.g. jackc/pgx on an internal context deadline) keeps the - server connection alive by running a self-cleanup batch such as - - SET SESSION AUTHORIZATION DEFAULT; - RESET ALL; - CLOSE ALL; - UNLISTEN *; - SELECT pg_advisory_unlock_all(); - DISCARD PLANS; - DISCARD SEQUENCES; - DISCARD TEMP; - - pg_doorman must recognise that the session is already clean and skip the - checkin-time `RESET ROLE; RESET ALL; ...` round-trip it would otherwise send. - Without this recognition every dangling client query triples the traffic to - PostgreSQL: the original (stuck) query, the client's self-reset batch, and a - redundant pg_doorman reset. - - Each scenario starts PostgreSQL with `log_statement = 'all'` and asserts the - absence or presence of pg_doorman's cleanup statements in the server log. - The `RESET ROLE` prefix is used as a marker because pg_doorman always prefixes - its cleanup batch with `RESET ROLE;` and clients from the real world do not - issue it. +Feature: Cleanup after client session reset commands + Client reset commands suppress the corresponding built-in cleanup. + RESET ALL and per-parameter RESET share a command tag, so both suppress + the built-in RESET ALL. Background: Given PostgreSQL started with options "-c log_statement=all -c logging_collector=off" and pg_hba.conf: @@ -61,24 +41,20 @@ Feature: Client session reset batch suppresses doorman-side cleanup [[pools.example_db_session.users]] username = "example_user_1" password = "" - pool_size = 2 + pool_size = 1 """ @client-session-reset-cleanup-pgx-batch - Scenario: pgx-style session reset batch does not trigger a second server cleanup + Scenario: A pgx-style reset batch does not trigger a second RESET ALL When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" # Warm the pool with a trivial query so that server auth and any startup # chatter is already in the log before we start asserting on it. And we send SimpleQuery "SELECT 1" to session "one" - And we sleep 100ms When we truncate PostgreSQL log # Exactly the batch jackc/pgx emits on an internal context deadline. And we send SimpleQuery "SET SESSION AUTHORIZATION DEFAULT; RESET ALL; CLOSE ALL; UNLISTEN *; SELECT pg_advisory_unlock_all(); DISCARD PLANS; DISCARD SEQUENCES; DISCARD TEMP" to session "one" - And we sleep 300ms - # Client batch itself still shows up once — that is the one we expect. + # ReadyForQuery arrives after checkin; only the client's RESET ALL is logged. Then PostgreSQL log should contain exactly 1 occurrences of "RESET ALL" - # pg_doorman's checkin cleanup would have prefixed its batch with RESET ROLE. - # Its absence proves the second cleanup was suppressed. And PostgreSQL log should not contain "RESET ROLE" @client-session-reset-cleanup-real-set-still-cleans @@ -139,23 +115,20 @@ Feature: Client session reset batch suppresses doorman-side cleanup And PostgreSQL log should not contain "RESET ROLE" @client-session-reset-cleanup-per-guc-reset - Scenario: Per-GUC RESET disarms set-cleanup - # PostgreSQL returns the same `RESET` tag for `RESET ALL` and `RESET foo`, - # so a per-GUC RESET after a SET on the same GUC leaves the session clean - # as far as pg_doorman is concerned. Documents the intentional trade-off: - # `SET a=1; SET b=2; RESET a;` would also be treated as clean, because - # pg_doorman only tracks a single cleanup bit and cannot distinguish which - # GUCs are still modified. + Scenario: Per-GUC RESET suppresses built-in RESET ALL When we create session "five" to pg_doorman as "example_user_1" with password "" and database "example_db_session" - And we send SimpleQuery "SELECT 1" to session "five" - And we sleep 100ms + And we send SimpleQuery "SELECT pg_backend_pid()" to session "five" and store backend_pid When we truncate PostgreSQL log And we send SimpleQuery "SET statement_timeout = 1000" to session "five" And we send SimpleQuery "RESET statement_timeout" to session "five" And we close session "five" - And we sleep 300ms - Then PostgreSQL log should contain "RESET statement_timeout" + # Pool size 1 makes the next checkout wait for the previous checkin. + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db_session" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "five" + And PostgreSQL log should contain "RESET statement_timeout" And PostgreSQL log should not contain "RESET ROLE" + And PostgreSQL log should contain exactly 0 occurrences of "RESET ALL" @client-session-reset-cleanup-single-close-keeps-armed Scenario: Closing one named cursor does not disarm declare-cleanup diff --git a/tests/bdd/features/copy-protocol.feature b/tests/bdd/features/copy-protocol.feature index 953685ce2..8ac30806e 100644 --- a/tests/bdd/features/copy-protocol.feature +++ b/tests/bdd/features/copy-protocol.feature @@ -110,25 +110,33 @@ Feature: COPY Protocol @copy-protocol-from-first Scenario: COPY FROM STDIN basic operation gives identical results When we login to postgres and pg_doorman as "example_user_1" with password "" and database "example_db" + # A temporary table does not survive a checkin in transaction pooling: + # hold one lease with an explicit transaction for the whole scenario. + And we send SimpleQuery "BEGIN" to both And we send SimpleQuery "CREATE TEMP TABLE copy_test_pg (id int, name text)" to both And we send CopyFromStdin "COPY copy_test_pg FROM STDIN" with data "1\ttest1\n2\ttest2\n3\ttest3\n" to both And we send SimpleQuery "SELECT * FROM copy_test_pg ORDER BY id" to both + And we send SimpleQuery "COMMIT" to both Then we should receive identical messages from both @copy-protocol-from-second Scenario: COPY FROM STDIN with CSV format gives identical results When we login to postgres and pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN" to both And we send SimpleQuery "CREATE TEMP TABLE copy_test_csv (id int, name text)" to both And we send CopyFromStdin "COPY copy_test_csv FROM STDIN WITH (FORMAT CSV)" with data "1,test1\n2,test2\n" to both And we send SimpleQuery "SELECT * FROM copy_test_csv ORDER BY id" to both + And we send SimpleQuery "COMMIT" to both Then we should receive identical messages from both @copy-protocol-from-third Scenario: COPY FROM STDIN with empty data gives identical results When we login to postgres and pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN" to both And we send SimpleQuery "CREATE TEMP TABLE copy_test_empty (id int, name text)" to both And we send CopyFromStdin "COPY copy_test_empty FROM STDIN" with data "" to both And we send SimpleQuery "SELECT COUNT(*) FROM copy_test_empty" to both + And we send SimpleQuery "COMMIT" to both Then we should receive identical messages from both @copy-protocol-from-error-first @@ -146,8 +154,10 @@ Feature: COPY Protocol @copy-protocol-from-error-third Scenario: COPY FROM STDIN with wrong column count gives identical error When we login to postgres and pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN" to both And we send SimpleQuery "CREATE TEMP TABLE copy_test_cols (id int, name text, value int)" to both And we send CopyFromStdin "COPY copy_test_cols FROM STDIN" with data "1\ttest\n" to both + And we send SimpleQuery "COMMIT" to both Then we should receive identical messages from both @copy-protocol-mixed-first diff --git a/tests/bdd/features/greengage-cleanup.feature b/tests/bdd/features/greengage-cleanup.feature new file mode 100644 index 000000000..be97933fb --- /dev/null +++ b/tests/bdd/features/greengage-cleanup.feature @@ -0,0 +1,331 @@ +@greengage @greengage-cleanup +Feature: Backend cleanup against a real Greengage coordinator and segments + Every scenario owns a database on the Docker Greengage cluster. + A pool of one and the next checkout form the cleanup completion barrier. + + Background: + Given a temporary Greengage database on the configured cluster + + @greengage-cleanup-built-in + Scenario: Built-in cleanup resets parameters, prepared statements and held cursors on the same backend + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + prepared_statements: true + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: session + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SET work_mem = '96MB'; PREPARE cleanup_stmt AS SELECT 42; BEGIN; DECLARE cleanup_cursor CURSOR WITH HOLD FOR SELECT 1; COMMIT" to session "first" + And we send SimpleQuery "SELECT current_setting('work_mem') = '96MB' AND EXISTS (SELECT 1 FROM pg_prepared_statements WHERE name = 'cleanup_stmt') AND EXISTS (SELECT 1 FROM pg_cursors WHERE name = 'cleanup_cursor')" to session "first" and store response + Then session "first" should receive exactly one DataRow "t" + When we send SimpleQuery "SELECT count(*), bool_and(setting = reset_val) FROM gp_dist_random('pg_settings') WHERE name = 'work_mem'" to session "first" and store response + Then session "first" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}|f" + When we send SimpleQuery "SELECT 1 / 0" to session "first" expecting error + Then session "first" should receive ErrorResponse with SQLSTATE "22012" + And session "first" should receive ReadyForQuery "I" + When we close session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT (SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem') AND NOT EXISTS (SELECT 1 FROM pg_prepared_statements) AND NOT EXISTS (SELECT 1 FROM pg_cursors)" to session "next" and store response + Then session "next" should receive exactly one DataRow "t" + And session "next" should receive only backend messages "TDCZ" + When we send SimpleQuery "SELECT count(*), bool_and(setting = reset_val) FROM gp_dist_random('pg_settings') WHERE name = 'work_mem'" to session "next" and store response + Then session "next" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}|t" + + @greengage-cleanup-full + Scenario: Explicit supported cleanup removes temporary tables from coordinator and all segments + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + cleanup_server_connections: always + cleanup_server_query: "SET SESSION AUTHORIZATION DEFAULT; RESET ALL; DEALLOCATE ALL; CLOSE ALL; UNLISTEN *; SELECT pg_advisory_unlock_all(); DISCARD PLANS; DISCARD SEQUENCES; DISCARD TEMP" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: session + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SET work_mem = '96MB'; CREATE TEMP TABLE cleanup_temp (id integer) DISTRIBUTED BY (id); INSERT INTO cleanup_temp VALUES (1), (2); PREPARE cleanup_stmt AS SELECT 42; BEGIN; DECLARE cleanup_cursor CURSOR WITH HOLD FOR SELECT 1; COMMIT" to session "first" + And we send SimpleQuery "SELECT current_setting('work_mem') = '96MB' AND to_regclass('pg_temp.cleanup_temp') IS NOT NULL AND EXISTS (SELECT 1 FROM pg_prepared_statements WHERE name = 'cleanup_stmt') AND EXISTS (SELECT 1 FROM pg_cursors WHERE name = 'cleanup_cursor')" to session "first" and store response + Then session "first" should receive exactly one DataRow "t" + When we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'cleanup_temp'" to session "first" and store response + Then session "first" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + When we close session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT (SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem') AND to_regclass('pg_temp.cleanup_temp') IS NULL AND NOT EXISTS (SELECT 1 FROM pg_prepared_statements) AND NOT EXISTS (SELECT 1 FROM pg_cursors)" to session "next" and store response + Then session "next" should receive exactly one DataRow "t" + And session "next" should receive only backend messages "TDCZ" + When we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'cleanup_temp'" to session "next" and store response + Then session "next" should receive exactly one DataRow "0" + + @greengage-cleanup-prepared + Scenario: An extended prepared statement reparses and restores startup parameters after supported cleanup + When we send SimpleQuery "CREATE SCHEMA startup_schema; CREATE TABLE startup_schema.values_table (value integer) DISTRIBUTED BY (value); INSERT INTO startup_schema.values_table VALUES (1)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + prepared_statements: true + sync_server_parameters: true + cleanup_server_connections: always + cleanup_server_query: "SET SESSION AUTHORIZATION DEFAULT; RESET ALL; DEALLOCATE ALL; CLOSE ALL; UNLISTEN *; SELECT pg_advisory_unlock_all(); DISCARD PLANS; DISCARD SEQUENCES; DISCARD TEMP" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: transaction + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" and startup parameters "search_path=startup_schema" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid as "before" + And we send Parse "cached" with query "SELECT value + $1::int FROM values_table" to session "one" + And we send Bind "" to "cached" with params "10" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive exactly one DataRow "11" + And session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT count(*) FROM pg_prepared_statements" to session "one" and store response + Then session "one" should receive exactly one DataRow "0" + When we send Bind "" to "cached" with params "11" to session "one" + And we send Execute "" to session "one" + And we send Sync to session "one" + Then session "one" should receive exactly one DataRow "12" + And session "one" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT pg_backend_pid()" to session "one" and store backend_pid as "after" + Then named backend_pid "after" from session "one" is same as "before" + + @greengage-discard-all-notice + Scenario: Real DISCARD ALL reports its partial cleanup and leaves temporary tables on segments + When we send SimpleQuery "SET work_mem = '96MB'; CREATE TEMP TABLE discard_temp (id integer) DISTRIBUTED BY (id); PREPARE discard_stmt AS SELECT 42" to session "observer" + And we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'discard_temp'" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + When we send SimpleQuery "DISCARD ALL" to session "observer" and store response + Then session "observer" should receive NoticeResponse with SQLSTATE "0AM01" + And session "observer" should receive CommandComplete "DISCARD ALL" + And session "observer" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT (SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem') AND NOT EXISTS (SELECT 1 FROM pg_prepared_statements) AND to_regclass('pg_temp.discard_temp') IS NULL" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "t" + When we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'discard_temp'" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + + @greengage-discard-all-reuse + Scenario: A real DISCARD ALL cleanup notice preserves backend reuse while segment cleanup remains incomplete + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + cleanup_server_connections: always + cleanup_server_query: "DISCARD ALL" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: session + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SET work_mem = '96MB'; CREATE TEMP TABLE discard_temp (id integer) DISTRIBUTED BY (id)" to session "first" + And we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'discard_temp'" to session "first" and store response + Then session "first" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + When we close session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT (SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem') AND to_regclass('pg_temp.discard_temp') IS NULL" to session "next" and store response + Then session "next" should receive exactly one DataRow "t" + And session "next" should receive only backend messages "TDCZ" + When we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'discard_temp'" to session "next" and store response + Then session "next" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + + @greengage-discard-all-suppressed + Scenario: client_min_messages suppresses the DISCARD ALL notice while segment cleanup remains incomplete + # Greengage emits 0AM01 before resetting client_min_messages. At warning, + # it sends no NoticeResponse; the command still leaves segment temporary tables. + When we send SimpleQuery "SET client_min_messages = warning" to session "observer" + And we send SimpleQuery "DISCARD ALL" to session "observer" and store response + Then session "observer" should receive only backend messages "CZ" + And session "observer" should receive CommandComplete "DISCARD ALL" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + cleanup_server_connections: always + cleanup_server_query: "DISCARD ALL" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: session + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SET client_min_messages = warning; CREATE TEMP TABLE suppressed_temp (id integer) DISTRIBUTED BY (id)" to session "first" + And we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'suppressed_temp'" to session "first" and store response + Then session "first" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + When we close session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT to_regclass('pg_temp.suppressed_temp') IS NULL" to session "next" and store response + Then session "next" should receive exactly one DataRow "t" + And session "next" should receive only backend messages "TDCZ" + When we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'suppressed_temp'" to session "next" and store response + Then session "next" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + + @greengage-user-function-notice + Scenario Outline: User function notices reach clients and preserve backend reuse when called by cleanup + When we send SimpleQuery "CREATE FUNCTION public.cleanup_notice(notice_code text) RETURNS integer LANGUAGE plpgsql AS $$ BEGIN RAISE NOTICE USING ERRCODE = notice_code, MESSAGE = 'application notice'; RETURN 7; END $$" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + cleanup_server_connections: always + cleanup_server_query: "RESET ALL; SELECT public.cleanup_notice('')" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: session + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + And we send SimpleQuery "SET work_mem = '96MB'" to session "first" + And we send SimpleQuery "SELECT current_setting('work_mem') = '96MB'" to session "first" and store response + Then session "first" should receive exactly one DataRow "t" + When we send SimpleQuery "SELECT public.cleanup_notice('')" to session "first" and store response + Then session "first" should receive NoticeResponse with SQLSTATE "" + And session "first" should receive exactly one DataRow "7" + And session "first" should receive ReadyForQuery "I" + When we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid as "after_notice" + Then backend_pid "after_notice" from session "first" should equal initial backend_pid from session "first" + When we close session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT setting = reset_val FROM pg_settings WHERE name = 'work_mem'" to session "next" and store response + Then session "next" should receive exactly one DataRow "t" + And session "next" should receive only backend messages "TDCZ" + + Examples: + | code | + | 0AM01 | + | 0A000 | + + @greengage-cleanup-commit + Scenario: A cleanup SQL error preserves the completed COMMIT response and committed rows + When we send SimpleQuery "CREATE TABLE public.committed_rows (id integer) DISTRIBUTED BY (id)" to session "observer" + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + cleanup_server_connections: always + cleanup_server_query: "SELECT 1 / 0" + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: transaction + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "one" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "before" + And we send SimpleQuery "INSERT INTO public.committed_rows VALUES (1)" to session "one" + And we send SimpleQuery "COMMIT" to session "one" and store response + Then session "one" should receive CommandComplete "COMMIT" + And session "one" should receive ReadyForQuery "I" + And session "one" should receive only backend messages "CZ" + When we send SimpleQuery "SELECT count(*) FROM public.committed_rows" to session "observer" and store response + Then session "observer" should receive exactly one DataRow "1" + When we send SimpleQuery "BEGIN; SELECT pg_backend_pid()" to session "one" and store backend_pid as "after" + Then named backend_pid "after" from session "one" is different from "before" + When we send SimpleQuery "SELECT count(*) FROM public.committed_rows" to session "one" and store response + Then session "one" should receive exactly one DataRow "1" + When we send SimpleQuery "ROLLBACK" to session "one" and store response + Then session "one" should receive CommandComplete "ROLLBACK" + And session "one" should receive ReadyForQuery "I" + + @greengage-cleanup-built-in-temp-tables + Scenario: Built-in DISCARD TEMP clears the temporary table on the coordinator and on the segments + Given pg_doorman started with config: + """ + general: + host: "127.0.0.1" + port: ${DOORMAN_PORT} + admin_username: admin + admin_password: admin + pg_hba: {content: "host all all 127.0.0.1/32 trust"} + pools: + example_db: + server_host: "${GREENGAGE_HOST}" + server_port: ${GREENGAGE_PORT} + server_database: "${GREENGAGE_DATABASE}" + pool_mode: session + users: [{username: example_user_1, password: "", server_username: "${GREENGAGE_USER}", server_password: "${GREENGAGE_PASSWORD}", pool_size: 1}] + """ + When we create session "first" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "first" and store backend_pid + # One batch is one lease: the temp table must exist here. After the + # checkin the built-in cleanup drops it, which the next session verifies. + And we send SimpleQuery "CREATE TEMP TABLE built_in_temp (id integer) DISTRIBUTED BY (id); SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'built_in_temp'" to session "first" and store response + Then session "first" should receive exactly one DataRow "${GREENGAGE_SEGMENTS}" + When we close session "first" + And we create session "next" to pg_doorman as "example_user_1" with password "" and database "example_db" + And we send SimpleQuery "SELECT pg_backend_pid()" to session "next" and store backend_pid + Then backend_pid from session "next" should equal backend_pid from session "first" + When we send SimpleQuery "SELECT to_regclass('pg_temp.built_in_temp') IS NULL" to session "next" and store response + Then session "next" should receive exactly one DataRow "t" + And session "next" should receive only backend messages "TDCZ" + When we send SimpleQuery "SELECT count(*) FROM gp_dist_random('pg_class') WHERE relname = 'built_in_temp'" to session "next" and store response + Then session "next" should receive exactly one DataRow "0" diff --git a/tests/bdd/features/prepared-statement-doesnt-exist-bug.feature b/tests/bdd/features/prepared-statement-doesnt-exist-bug.feature index 855081dd1..21348191a 100644 --- a/tests/bdd/features/prepared-statement-doesnt-exist-bug.feature +++ b/tests/bdd/features/prepared-statement-doesnt-exist-bug.feature @@ -20,7 +20,8 @@ Feature: Prepared statement cache desync on client disconnect before Sync """ And fixtures from "tests/fixture.sql" applied - Scenario: Client disconnect after Parse without Sync causes stale server cache + @cleanup-server-query-pending-parse + Scenario Outline: Client disconnect after Parse without Sync causes stale server cache Given pg_doorman started with config: """ [general] @@ -36,6 +37,8 @@ Feature: Prepared statement cache desync on client disconnect before Sync server_host = "127.0.0.1" server_port = ${PG_PORT} pool_mode = "transaction" + + [[pools.example_db.users]] username = "example_user_1" @@ -55,6 +58,11 @@ Feature: Prepared statement cache desync on client disconnect before Sync And we send Sync to session "two" Then session "two" should receive DataRow with "30" + Examples: + | reset_mode | reset_config | + | # adaptive | # selective cleanup | + | cleanup_server_connections = "always" | cleanup_server_query = "DISCARD ALL" | + Scenario: TCP abort after Parse without Sync causes stale server cache Given pg_doorman started with config: """ diff --git a/tests/bdd/greengage_helper.rs b/tests/bdd/greengage_helper.rs new file mode 100644 index 000000000..6edec8e39 --- /dev/null +++ b/tests/bdd/greengage_helper.rs @@ -0,0 +1,193 @@ +//! Opt-in BDD fixture for the real, externally managed Greengage cluster. +//! Each scenario owns one database; the Docker service owns the cluster. + +use crate::extended::helpers::{parse_datarow_fields, ProtocolMessages}; +use crate::pg_connection::PgConnection; +use crate::world::DoormanWorld; +use cucumber::{given, then}; +use std::time::Duration; +use tokio::time::{interval, timeout}; + +pub struct GreengageDatabase { + database: String, + admin: PgConnection, +} + +async fn connect(addr: &str, user: &str, password: &str, database: &str) -> PgConnection { + timeout(Duration::from_secs(15), async { + let mut connection = PgConnection::connect(addr) + .await + .expect("Failed to connect to the Greengage coordinator"); + connection + .send_startup(user, database) + .await + .expect("Failed to send Greengage startup"); + connection + .authenticate(user, password) + .await + .expect("Failed to authenticate to Greengage"); + connection + }) + .await + .expect("Timed out connecting to Greengage") +} + +async fn query(connection: &mut PgConnection, sql: &str) -> ProtocolMessages { + let messages = timeout(Duration::from_secs(15), async { + connection.send_simple_query(sql).await?; + connection.read_all_messages_until_ready().await + }) + .await + .unwrap_or_else(|_| panic!("Greengage fixture query timed out: {sql}")) + .unwrap_or_else(|error| panic!("Greengage fixture query failed: {sql}: {error}")); + for (tag, body) in &messages { + assert_ne!( + *tag, + 'E', + "Greengage fixture SQL failed: {sql}: {}", + String::from_utf8_lossy(body) + ); + } + messages +} + +fn first_row(messages: &ProtocolMessages) -> Vec { + messages + .iter() + .find(|(tag, _)| *tag == 'D') + .map(|(_, body)| parse_datarow_fields(body)) + .expect("Greengage fixture query returned no row") +} + +#[given("a temporary Greengage database on the configured cluster")] +pub async fn create_database(world: &mut DoormanWorld) { + let host = std::env::var("GREENGAGE_HOST").unwrap_or_else(|_| "127.0.0.1".into()); + let port = std::env::var("GREENGAGE_PORT") + .expect("Set GREENGAGE_PORT to run @greengage scenarios") + .parse::() + .expect("GREENGAGE_PORT must be a TCP port"); + let user = std::env::var("GREENGAGE_USER").unwrap_or_else(|_| "gpadmin".into()); + let password = std::env::var("GREENGAGE_PASSWORD").unwrap_or_else(|_| "greengage-test".into()); + let addr = format!("{host}:{port}"); + let mut admin = connect(&addr, &user, &password, "postgres").await; + let version = first_row(&query(&mut admin, "SELECT version()").await).remove(0); + assert!( + version.contains("Greengage"), + "@greengage requires a real Greengage server, got: {version}" + ); + let segments = first_row( + &query( + &mut admin, + "SELECT count(*) FROM gp_segment_configuration WHERE content >= 0 AND role = 'p' AND status = 'u'", + ) + .await, + ) + .remove(0); + assert!( + segments.parse::().expect("Invalid segment count") >= 2, + "Greengage BDD requires at least two running primary segments" + ); + + // Generated identifiers contain only ASCII letters, digits and underscores. + let database = format!( + "doorman_bdd_{}_{:016x}", + std::process::id(), + rand::random::() + ); + world.greengage_database = Some(GreengageDatabase { + database: database.clone(), + admin, + }); + query( + &mut world.greengage_database.as_mut().unwrap().admin, + &format!("CREATE DATABASE {database}"), + ) + .await; + for (name, value) in [ + ("GREENGAGE_HOST", host), + ("GREENGAGE_PORT", port.to_string()), + ("GREENGAGE_DATABASE", database.clone()), + ("GREENGAGE_USER", user.clone()), + ("GREENGAGE_PASSWORD", password.clone()), + ("GREENGAGE_SEGMENTS", segments), + ] { + world.vars.insert(name.into(), value); + } + let observer = connect(&addr, &user, &password, &database).await; + world.named_sessions.insert("observer".into(), observer); + println!("Greengage fixture: {version}; database {database}"); +} + +/// Called after pg_doorman is stopped, including when a scenario failed. +pub async fn drop_database(world: &mut DoormanWorld) { + let Some(mut fixture) = world.greengage_database.take() else { + return; + }; + world.named_sessions.clear(); + world.pg_conn = None; + world.doorman_conn = None; + timeout(Duration::from_secs(20), async { + let mut poll = interval(Duration::from_millis(50)); + loop { + query( + &mut fixture.admin, + &format!( + "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{}' AND pid <> pg_backend_pid()", + fixture.database + ), + ) + .await; + let active = first_row( + &query( + &mut fixture.admin, + &format!( + "SELECT count(*) FROM pg_stat_activity WHERE datname = '{}'", + fixture.database + ), + ) + .await, + ); + if active[0] == "0" { + break; + } + poll.tick().await; + } + query( + &mut fixture.admin, + &format!("DROP DATABASE IF EXISTS {}", fixture.database), + ) + .await; + }) + .await + .expect("Timed out removing the scenario's Greengage database"); +} + +fn sqlstate(body: &[u8]) -> Option<&str> { + let mut fields = body.split(|byte| *byte == 0); + fields.find_map(|field| { + (field.first() == Some(&b'C')) + .then(|| std::str::from_utf8(&field[1..]).ok()) + .flatten() + }) +} + +#[then(regex = r#"^session "([^"]+)" should receive NoticeResponse with SQLSTATE "([^"]+)"$"#)] +pub async fn notice_with_sqlstate(world: &mut DoormanWorld, session: String, expected: String) { + let messages = &world.session_messages[&session]; + assert!( + messages + .iter() + .any(|(tag, body)| *tag == 'N' && sqlstate(body) == Some(expected.as_str())), + "Session {session}: expected NoticeResponse with SQLSTATE {expected}, got {messages:?}" + ); +} + +#[then(regex = r#"^session "([^"]+)" should receive only backend messages "([A-Za-z0-9]+)"$"#)] +pub async fn exact_message_tags(world: &mut DoormanWorld, session: String, expected: String) { + let messages = &world.session_messages[&session]; + let actual: String = messages.iter().map(|(tag, _)| *tag).collect(); + assert_eq!( + actual, expected, + "Session {session}: unexpected protocol messages: {messages:?}" + ); +} diff --git a/tests/bdd/main.rs b/tests/bdd/main.rs index f33b950db..81a29bfd6 100644 --- a/tests/bdd/main.rs +++ b/tests/bdd/main.rs @@ -7,6 +7,7 @@ mod fallback_helper; mod fuzz_client; mod fuzz_helper; mod generate_helper; +mod greengage_helper; mod mock_patroni_helper; mod odyssey_helper; mod pg_connection; @@ -62,6 +63,20 @@ fn main() { #[cfg(target_os = "linux")] let base_filter = not_todo_skip; + // Ordinary BDD runs do not require the optional Greengage service. + // Supplying its port opts in while the Given step verifies its identity. + let base_filter = + if std::env::var_os("GREENGAGE_PORT").is_some() || cli.tags_filter.is_some() { + base_filter + } else { + TagOperation::And( + Box::new(base_filter), + Box::new(TagOperation::Not(Box::new(TagOperation::Tag( + "greengage".to_string(), + )))), + ) + }; + // Combine with existing tags filter if present cli.tags_filter = match cli.tags_filter.take() { Some(existing) => Some(TagOperation::And(Box::new(existing), Box::new(base_filter))), @@ -103,10 +118,10 @@ fn main() { world.slow_warning_abort = Some(slow_warning_task.abort_handle()); }) }) - .after(|_feature, _rule, _scenario, _finished, world| { + .after(|_feature, _rule, _scenario, _finished, mut world| { // NOTE: We only stop the specific process from this scenario, NOT all pg_doorman processes // because the next scenario's Background steps may have already started a new pg_doorman - if let Some(w) = world { + if let Some(w) = world.as_deref_mut() { if let Some(abort_handle) = w.slow_warning_abort.take() { abort_handle.abort(); } @@ -136,7 +151,11 @@ fn main() { } w.odyssey_process = None; } - Box::pin(async {}) + Box::pin(async move { + if let Some(w) = world { + greengage_helper::drop_database(w).await; + } + }) }) .run("tests/bdd/features") .await; diff --git a/tests/bdd/pool_bench_helper.rs b/tests/bdd/pool_bench_helper.rs index 3fa8736fe..e95a5a707 100644 --- a/tests/bdd/pool_bench_helper.rs +++ b/tests/bdd/pool_bench_helper.rs @@ -15,7 +15,7 @@ fn is_pprof_enabled() -> bool { std::env::var("PPROF").map(|v| v == "1").unwrap_or(false) } -use pg_doorman::config::{Address, User}; +use pg_doorman::config::{Address, CleanupMode, User}; use pg_doorman::pool::{ ClientServerMap, Pool, PoolConfig, QueueMode, ScalingConfig, ScalingStatsSnapshot, ServerPool, Timeouts, @@ -63,9 +63,10 @@ async fn setup_internal_pool(world: &mut DoormanWorld, size: usize, _mode: Strin user, "postgres", client_server_map, - true, // cleanup_connections - false, // log_client_parameter_status_changes - 0, // prepared_statement_cache_size + CleanupMode::Adaptive, // cleanup_connections + None, // cleanup_server_query + false, // log_client_parameter_status_changes + 0, // prepared_statement_cache_size "pool_bench".to_string(), 4, // max_concurrent_creates 0, // lifetime_ms (0 = unlimited) @@ -417,7 +418,8 @@ async fn setup_internal_pool_with_lifetimes( user, "postgres", client_server_map, - true, + CleanupMode::Adaptive, + None, false, 0, "pool_bench".to_string(), diff --git a/tests/bdd/world.rs b/tests/bdd/world.rs index 966f4eb90..41f6109de 100644 --- a/tests/bdd/world.rs +++ b/tests/bdd/world.rs @@ -22,6 +22,8 @@ pub struct TestCommandResult { /// The World struct holds the state shared across all steps in a scenario. #[derive(Default, World)] pub struct DoormanWorld { + /// Per-scenario database on the opt-in, externally managed Greengage cluster. + pub greengage_database: Option, /// Temporary directory for PostgreSQL data pub pg_tmp_dir: Option, /// PostgreSQL port diff --git a/tests/nix/greengage.sh b/tests/nix/greengage.sh new file mode 100644 index 000000000..b9dbf1b0c --- /dev/null +++ b/tests/nix/greengage.sh @@ -0,0 +1,128 @@ +#!/usr/bin/env bash + +# Sourced by run-tests.sh. The official image contains compiled Greengage and +# its demo-cluster scripts; no database compilation is needed at startup. +GREENGAGE_LIBRARY_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +GREENGAGE_IMAGE="${GREENGAGE_IMAGE:-greengagedb/ggdb7_ubuntu:7.5.0@sha256:5176ca76baf6f33092665454b3669310157c42af11ccf4f539f3252aee8ea7c3}" + +greengage_bootstrap() { + set -eo pipefail + cd /home/gpadmin + # Use the installation and account setup shipped in the pinned image. + # shellcheck source=/dev/null + source gpdb_src/concourse/scripts/common.bash + install_gpdb + gpdb_src/concourse/scripts/setup_gpadmin_user.bash + su gpadmin -c 'source /usr/local/greengage-db-devel/greengage_path.sh; LANG=en_US.UTF-8 make -C /home/gpadmin/gpdb_src/gpAux/gpdemo create-demo-cluster PORT_BASE=5432 NUM_PRIMARY_MIRROR_PAIRS=2 WITH_MIRRORS=false WITH_STANDBY=false' + + # These credentials belong only to this disposable test cluster. Explicit + # MD5 authentication exercises the backend auth path supported by doorman. + su gpadmin -c 'source /usr/local/greengage-db-devel/greengage_path.sh; psql -X -v ON_ERROR_STOP=1 -p 5432 -d postgres' <<'SQL' +SET password_encryption = 'md5'; +ALTER ROLE gpadmin PASSWORD 'greengage-test'; +SQL + local coordinator=/home/gpadmin/gpdb_src/gpAux/gpdemo/datadirs/qddir/demoDataDir-1 + { + printf '%s\n' 'host all all 127.0.0.1/32 md5' 'host all all ::1/128 md5' + cat "${coordinator}/pg_hba.conf" + } > "${coordinator}/pg_hba.conf.test" + mv "${coordinator}/pg_hba.conf.test" "${coordinator}/pg_hba.conf" + chown gpadmin:gpadmin "${coordinator}/pg_hba.conf" + su gpadmin -c 'source /usr/local/greengage-db-devel/greengage_path.sh; psql -X -v ON_ERROR_STOP=1 -p 5432 -d postgres -c "SELECT pg_reload_conf()"' + touch /tmp/greengage-bootstrap-ready + exec sleep infinity +} + +stop_greengage() { + if [ -z "${GREENGAGE_CONTAINER:-}" ] || [ -z "${GREENGAGE_OWNER_TOKEN:-}" ]; then + return 0 + fi + local owner + owner=$(docker inspect --format '{{ index .Config.Labels "org.pg-doorman.greengage-test-owner" }}' "${GREENGAGE_CONTAINER}" 2>/dev/null) || return 0 + if [ "${owner}" = "${GREENGAGE_OWNER_TOKEN}" ]; then + docker rm --force --volumes "${GREENGAGE_CONTAINER}" >/dev/null + fi + unset GREENGAGE_CONTAINER GREENGAGE_OWNER_TOKEN +} + +greengage_start_failure() { + printf '[ERROR] Greengage startup failed: %s\n' "$1" >&2 + docker logs --tail 200 "${GREENGAGE_CONTAINER}" >&2 || true + stop_greengage + return 1 +} + +start_greengage() { + if [ -n "${GREENGAGE_CONTAINER:-}" ]; then + printf '[ERROR] Greengage is already allocated by this runner\n' >&2 + return 1 + fi + local startup_timeout="${GREENGAGE_STARTUP_TIMEOUT:-600}" + case "${startup_timeout}" in + ''|*[!0-9]*|0) + printf '[ERROR] GREENGAGE_STARTUP_TIMEOUT must be a positive number of seconds\n' >&2 + return 1 + ;; + esac + export GREENGAGE_HOST=127.0.0.1 GREENGAGE_PORT=5432 + export GREENGAGE_USER=gpadmin GREENGAGE_PASSWORD=greengage-test + # Set ownership before creating the container so EXIT cleanup also covers + # interrupted startup. The label prevents deleting an unrelated container. + GREENGAGE_OWNER_TOKEN="$$-${RANDOM}-${RANDOM}" + GREENGAGE_CONTAINER="pg-doorman-greengage-${GREENGAGE_OWNER_TOKEN}" + export GREENGAGE_CONTAINER + printf '[INFO] Starting Greengage: %s\n' "${GREENGAGE_IMAGE}" + if ! docker create \ + --name "${GREENGAGE_CONTAINER}" \ + --label "org.pg-doorman.greengage-test-owner=${GREENGAGE_OWNER_TOKEN}" \ + --platform linux/amd64 \ + --init \ + --shm-size 512m \ + --ulimit nofile=65536:65536 \ + --sysctl 'kernel.sem=500 1024000 200 4096' \ + -v "${GREENGAGE_LIBRARY_DIR}/greengage.sh:/tmp/pg-doorman-greengage.sh:ro" \ + "${GREENGAGE_IMAGE}" bash /tmp/pg-doorman-greengage.sh --bootstrap >/dev/null; then + greengage_start_failure 'container creation failed' + return 1 + fi + if ! docker start "${GREENGAGE_CONTAINER}" >/dev/null; then + greengage_start_failure 'container start failed' + return 1 + fi + + local deadline=$((SECONDS + startup_timeout)) + local ready running + while [ "${SECONDS}" -lt "${deadline}" ]; do + running=$(docker inspect --format '{{.State.Running}}' "${GREENGAGE_CONTAINER}") || running=false + if [ "${running}" != true ]; then + greengage_start_failure 'bootstrap exited before the cluster became ready' + return 1 + fi + # Check authentication, catalog health, and a query actually dispatched + # to both primary segments, rather than accepting a listening socket. + ready=$(docker exec \ + -e PGPASSWORD="${GREENGAGE_PASSWORD}" \ + -e PGCONNECT_TIMEOUT=2 \ + -e 'PGOPTIONS=-c statement_timeout=5000' \ + "${GREENGAGE_CONTAINER}" timeout 8 bash -c ' + test -f /tmp/greengage-bootstrap-ready || exit 1 + source /usr/local/greengage-db-devel/greengage_path.sh + psql -X -A -t -v ON_ERROR_STOP=1 -h 127.0.0.1 -p 5432 -U gpadmin -d postgres -c "SELECT version() LIKE '\''%Greengage Database 7.%'\'' AND (SELECT count(*) >= 2 AND bool_and(status = '\''u'\'') FROM gp_segment_configuration WHERE content >= 0 AND role = '\''p'\'') AND (SELECT count(*) >= 2 FROM gp_dist_random('\''gp_id'\''));" + ' 2>/dev/null) || ready= + if [ "${ready}" = t ]; then + printf '[INFO] Greengage coordinator and two primary segments are ready\n' + return 0 + fi + sleep 2 + done + greengage_start_failure "cluster did not become ready within ${startup_timeout}s" +} + +if [ "${BASH_SOURCE[0]}" = "$0" ]; then + if [ "${1:-}" = --bootstrap ]; then + greengage_bootstrap + else + printf 'This file is a library for run-tests.sh.\n' >&2 + exit 1 + fi +fi diff --git a/tests/nix/run-tests.sh b/tests/nix/run-tests.sh index 21fde19ef..2329b5029 100755 --- a/tests/nix/run-tests.sh +++ b/tests/nix/run-tests.sh @@ -88,12 +88,29 @@ run_in_container() { --init -v "${PROJECT_ROOT}:/workspace" -w /workspace - --network host - --tmpfs /tmp:exec,mode=1777 + --network "${GREENGAGE_CONTAINER:+container:}${GREENGAGE_CONTAINER:-host}" + --tmpfs "/tmp:exec,mode=1777" -e "POSTGRES_HOST=127.0.0.1" -e "POSTGRES_PORT=5432" ) + if [ -n "${GREENGAGE_CONTAINER:-}" ]; then + docker_args+=( + -e "GREENGAGE_HOST=${GREENGAGE_HOST}" + -e "GREENGAGE_PORT=${GREENGAGE_PORT}" + -e "GREENGAGE_USER=${GREENGAGE_USER}" + -e "GREENGAGE_PASSWORD=${GREENGAGE_PASSWORD}" + ) + fi + + # CI reuses its prebuilt cargo cache; local runs can separate Linux artifacts. + local cargo_var + for cargo_var in CARGO_HOME CARGO_TARGET_DIR CARGO_NET_RETRY CARGO_HTTP_TIMEOUT CARGO_NET_OFFLINE; do + if [ -n "${!cargo_var:-}" ]; then + docker_args+=(-e "${cargo_var}=${!cargo_var}") + fi + done + # Add -t for colored output if stdout is a terminal if [ -t 1 ]; then docker_args+=(-t) @@ -161,7 +178,9 @@ run_bdd_tests() { fi cmd="${cmd} --test bdd" if [ -n "$tags" ]; then - cmd="${cmd} -- --tags ${tags}" + local quoted_tags + printf -v quoted_tags '%q' "$tags" + cmd="${cmd} -- --tags ${quoted_tags}" fi run_in_container "$cmd" @@ -220,6 +239,7 @@ Commands: build Build pg_doorman inside container bdd [tags] Run BDD/Cucumber tests (optionally with tags like @go, @python) + greengage [tags] Run BDD against a disposable Greengage cluster (default: @greengage) test-go Run Go client BDD tests test-rust Run Rust client BDD tests test-python Run Python client BDD tests @@ -234,6 +254,8 @@ Environment variables: REPO Repository name (auto-detected from git) IMAGE_TAG Image tag to use (default: latest) DEBUG Enable debug output + GREENGAGE_IMAGE Override the pinned Greengage Docker image + CARGO_TARGET_DIR Override the target directory inside the test container BENCHER_API_TOKEN API token for bencher.dev (for benchmark reporting) BENCHER_PROJECT Bencher project name (default: pg-doorman) BENCHER_BRANCH Bencher branch name (default: main) @@ -244,6 +266,7 @@ Examples: $0 shell # Interactive shell $0 build # Build pg_doorman $0 bdd @go # Run BDD tests tagged with @go + $0 greengage # Run Greengage cleanup tests in the Nix test runner $0 test-rust # Run Rust client tests EOF @@ -266,6 +289,16 @@ case "${1:-help}" in try_pull_image run_bdd_tests "${2:-}" ;; + greengage) + try_pull_image + # shellcheck source=greengage.sh + source "${SCRIPT_DIR}/greengage.sh" + trap stop_greengage EXIT + trap 'exit 130' INT + trap 'exit 143' TERM + start_greengage + run_bdd_tests "${2:-@greengage}" + ;; test-go) try_pull_image test_go