proxysql

Commit Graph

Author	SHA1	Message	Date
Rene Cannao	f776af5487	test(pgsql): harden tx_poisoned tests after review Driven by the test-coverage review findings. Seven substantive changes across both test files: 1. NoticeResponse payload is now inspected (review 1.1/1.2). Install PQsetNoticeReceiver with a NoticeBag collector. Assert the poison NoticeResponse has severity=WARNING, sqlstate=01000, and non-empty message; and that NO notice carries sqlstate 57P01 (backend-leak) or 00000 (wrong pre-fix code). Without this, the no-57P01-leak invariant called out in issue #5658 was unverified. 2. COMMIT-on-poisoned notice payload is inspected (review 1.4 / 3.3). Case B now asserts the 25P01 "there is no transaction in progress" notice is emitted alongside the synthesized ROLLBACK command tag — the other half of the PG-native parity contract. 3. ROLLBACK TO SAVEPOINT, AND CHAIN, multi-statement, and ROLLBACKET word-boundary are exercised (review 1.3/1.14). Added as a batch alongside SELECT 42 and RELEASE SAVEPOINT, each asserted to return 25P02 and stay poisoned. 4. Autocommit-no-poison (case C) + admin-off-no-counter-move (case D) are now explicit (review 1.8 / 2.4). Pre-fix behavior: a non-tx statement killed mid-flight bumped the total counter because the feature used IsActiveTransaction() (polluted by the disconnect heuristic). Case C confirms the is_in_transaction() tightening. Case D confirms that toggling the admin var off truly disables the feature (no counter movement), not just preserves client termination. 5. Counter deltas are EXACT (==), not >= (review 2.1). Recovery test's +1/+1/+6 and ext-query test's +3 are all equality-checked. A bug that over-counts (e.g. regressed to per-wire-packet accounting) would be caught. 6. Flakiness-resistance (review 4.1/4.2/4.3/4.4/5.3). The killer no longer sleeps 500 ms blindly; it polls pg_stat_activity up to 6 s for the marker backend and reports a clean diag-plus-skip if the kill didn't fire. This is the same pattern used by the sibling retry_guard test. kill_delivered is now surfaced on the return struct so the caller can distinguish "feature regressed" from "CI infra flaked". 7. Admin-variable hygiene (review 5.1/6.4). - RAII-scoped RestorePreserveClientAdminVar: restore to default (=true) on every exit path including failure. - UPDATE now uses the full 'pgsql-<name>' prefix on variable_name; the previous WHERE clause matched zero rows and silently left the runtime value unchanged (which is how the pre-fix case D appeared to pass PQstatus checks while counters still moved). - After LOAD PGSQL VARIABLES TO RUNTIME, open a burst of short- lived PGconns to trigger activity on every PgSQL worker, guaranteeing each thread's next poll() wake-up picks up the new __thread-local admin-var value. Eliminates the up-to-2s refresh lag (pgsql-poll_timeout) that otherwise leaves stale worker state when case D runs. Severity / severity (ext-query test only): the rejected_25P02_as_error() predicate now also checks severity=="ERROR", catching a regression that might emit FATAL 25P02 (libpq would close the socket on FATAL despite the 25P02 code). Verified: both tests pass 17/17 and 11/11 against a live legacy-g2 infra. Case D cleanly shows counters unchanged when admin is off; case C cleanly shows counters unchanged for an autocommit kill.	2 months ago
Rene Cannao	100e417179	test(pgsql): extended-query coverage for tx_poisoned behavior New sibling to pgsql-tx_poisoned_recovery-t, plus a one-line semantics fix in the extended-query poison gate. Semantics fix (lib/PgSQL_Session.cpp): Before, the pgsql_tx_poisoned_rejected_statements_total counter was incremented on EVERY swallowed ext-query wire packet — one per Parse / Bind / Describe / Close / Execute / Sync. That made a single logical PQexecParams call count as 4+, inconsistent with the simple-query path where a 'Q' packet bumps it exactly once. Now P/B/D/C/E are silently swallowed (no counter bump), and Sync is the single accounting point — the counter value matches the number of client-visible "rejected statements" regardless of protocol. New test pgsql-tx_poisoned_extended_query-t (plan(11)): * Poison the session via a simple-query pg_sleep + mid-tx kill. * While poisoned: - PQexecParams("SELECT 42", 0 params) returns 25P02. - PQprepare("stmt_poisoned", "SELECT 43") returns 25P02. - PQexecPrepared on that name returns 25P02. * The rejected counter increments by exactly 3 (one per logical ext-query call), not by the 8-11 wire packets that libpq actually sends. This assertion IS the mechanical proof that the semantics fix above took effect. * Simple-query ROLLBACK recovers the session. * Post-recovery PQexecParams and PQprepare+PQexecPrepared both execute cleanly and return rows. Registered alongside the simple-query sibling in legacy-g2. Verified against a live legacy-g2 infra: both tests pass (11/11 and 15/15), counter deltas exactly match expected per-statement semantics.	2 months ago

Author

SHA1

Message

Date

Rene Cannao

f776af5487

test(pgsql): harden tx_poisoned tests after review

Driven by the test-coverage review findings. Seven substantive
changes across both test files:

1. NoticeResponse payload is now inspected (review 1.1/1.2).
   Install PQsetNoticeReceiver with a NoticeBag collector. Assert
   the poison NoticeResponse has severity=WARNING, sqlstate=01000,
   and non-empty message; and that NO notice carries sqlstate
   57P01 (backend-leak) or 00000 (wrong pre-fix code). Without
   this, the no-57P01-leak invariant called out in issue #5658
   was unverified.

2. COMMIT-on-poisoned notice payload is inspected (review 1.4 / 3.3).
   Case B now asserts the 25P01 "there is no transaction in
   progress" notice is emitted alongside the synthesized ROLLBACK
   command tag — the other half of the PG-native parity contract.

3. ROLLBACK TO SAVEPOINT, AND CHAIN, multi-statement, and
   ROLLBACKET word-boundary are exercised (review 1.3/1.14).
   Added as a batch alongside SELECT 42 and RELEASE SAVEPOINT,
   each asserted to return 25P02 and stay poisoned.

4. Autocommit-no-poison (case C) + admin-off-no-counter-move
   (case D) are now explicit (review 1.8 / 2.4).
   Pre-fix behavior: a non-tx statement killed mid-flight bumped
   the total counter because the feature used IsActiveTransaction()
   (polluted by the disconnect heuristic). Case C confirms the
   is_in_transaction() tightening. Case D confirms that toggling
   the admin var off truly disables the feature (no counter
   movement), not just preserves client termination.

5. Counter deltas are EXACT (==), not >= (review 2.1).
   Recovery test's +1/+1/+6 and ext-query test's +3 are all
   equality-checked. A bug that over-counts (e.g. regressed to
   per-wire-packet accounting) would be caught.

6. Flakiness-resistance (review 4.1/4.2/4.3/4.4/5.3).
   The killer no longer sleeps 500 ms blindly; it polls
   pg_stat_activity up to 6 s for the marker backend and reports
   a clean diag-plus-skip if the kill didn't fire. This is the
   same pattern used by the sibling retry_guard test. kill_delivered
   is now surfaced on the return struct so the caller can distinguish
   "feature regressed" from "CI infra flaked".

7. Admin-variable hygiene (review 5.1/6.4).
   - RAII-scoped RestorePreserveClientAdminVar: restore to default
     (=true) on every exit path including failure.
   - UPDATE now uses the full 'pgsql-<name>' prefix on
     variable_name; the previous WHERE clause matched zero rows
     and silently left the runtime value unchanged (which is how
     the pre-fix case D appeared to pass PQstatus checks while
     counters still moved).
   - After LOAD PGSQL VARIABLES TO RUNTIME, open a burst of short-
     lived PGconns to trigger activity on every PgSQL worker,
     guaranteeing each thread's next poll() wake-up picks up
     the new __thread-local admin-var value. Eliminates the
     up-to-2s refresh lag (pgsql-poll_timeout) that otherwise
     leaves stale worker state when case D runs.

Severity / severity (ext-query test only): the rejected_25P02_as_error()
predicate now also checks severity=="ERROR", catching a regression that
might emit FATAL 25P02 (libpq would close the socket on FATAL despite
the 25P02 code).

Verified: both tests pass 17/17 and 11/11 against a live legacy-g2
infra. Case D cleanly shows counters unchanged when admin is off;
case C cleanly shows counters unchanged for an autocommit kill.

Rene Cannao

100e417179

test(pgsql): extended-query coverage for tx_poisoned behavior

New sibling to pgsql-tx_poisoned_recovery-t, plus a one-line semantics
fix in the extended-query poison gate.

Semantics fix (lib/PgSQL_Session.cpp):
  Before, the pgsql_tx_poisoned_rejected_statements_total counter was
  incremented on EVERY swallowed ext-query wire packet — one per Parse /
  Bind / Describe / Close / Execute / Sync. That made a single logical
  PQexecParams call count as 4+, inconsistent with the simple-query
  path where a 'Q' packet bumps it exactly once.
  Now P/B/D/C/E are silently swallowed (no counter bump), and Sync is
  the single accounting point — the counter value matches the number
  of client-visible "rejected statements" regardless of protocol.

New test pgsql-tx_poisoned_extended_query-t (plan(11)):
  * Poison the session via a simple-query pg_sleep + mid-tx kill.
  * While poisoned:
      - PQexecParams("SELECT 42", 0 params) returns 25P02.
      - PQprepare("stmt_poisoned", "SELECT 43") returns 25P02.
      - PQexecPrepared on that name returns 25P02.
  * The rejected counter increments by exactly 3 (one per logical
    ext-query call), not by the 8-11 wire packets that libpq actually
    sends. This assertion IS the mechanical proof that the semantics
    fix above took effect.
  * Simple-query ROLLBACK recovers the session.
  * Post-recovery PQexecParams and PQprepare+PQexecPrepared both
    execute cleanly and return rows.

Registered alongside the simple-query sibling in legacy-g2. Verified
against a live legacy-g2 infra: both tests pass (11/11 and 15/15),
counter deltas exactly match expected per-statement semantics.

2 Commits (feature/pgsql-native-backend-protocol)