PR #6522 - sysown/proxysql

Four bugs with query cancels and the wait for a backend connection.

1. A cancel for an idle session made the worker thread busy-poll (#6521)

A query cancel (pg_cancel_backend() or a CancelRequest) ends in Scan_Sessions_to_Kill___handle_query_cancellation(). It set cancel_query on the session's backend data stream whatever the session was doing. While the flag is set, ProcessAllMyDS_BeforePoll() gives poll() a 0 ms timeout, and only a running query cleared it. So the worker thread busy-polled:

for a session idle inside a transaction, until the client sent its next query;
for a fast-forward session, until the connection closed.

Fix (d238c994e): the flag is set only when the session is not in WAITING_CLIENT_DATA. An idle session has nothing to cancel, and PostgreSQL ignores the request too.

2. A cancel in fast forward never reached the backend

Only the query handler sent the cancel, so in fast forward the query ran to the end. That hit fast-forward users, and any user during COPY ... FROM STDIN, which runs in fast forward.

Fix (8d9198644): the fast-forward handler sends every cancel to the backend, which ignores it when no query is running. It resets killed_at, so later cancels in the same session are sent too.

3. A cancel while a statement waited for a backend connection was lost

When no connection was free in the hostgroup, handler_again___status_CONNECTING_SERVER() never read cancel_query. In native mode the extended-query batch built after the wait cleared it, so the cancel was lost. In libpq mode it reached the backend only if the statement was still running by then. Either way the statement ran once a connection freed up, and an INSERT committed although the client had cancelled it.

The connect-timeout exit and the connect-failure exit had two related gaps. They left the rest of an extended-query unit queued, so the client's next statement failed with 26000. Inside a transaction they told the client it was idle, so a later COMMIT could save work done before the failed statement.

Fix (52ebc4699): the wait ends with 57014 when the cancel flag is set (not in fast forward or for a mirror session). The cancel and the timeout share one exit, give_up_waiting(): it fails an open transaction, drops the rest of the unit, ends the request and closes a half-open connection. The connect-failure exit also fails the transaction and drops the rest of the unit.

Not covered: a cancel during session-variable sync or during a query rule's delay. Those are separate waits.

4. Two ReadyForQuery after a failed connection wait before an implicit Sync

Found in review. When a simple Query arrives while an extended-query unit is pending, ProxySQL adds the Sync itself, and the client expects one ReadyForQuery, after the Query. If the unit's connection wait ended on a timeout or a refused connection, the error came with its own ReadyForQuery, so the client got two and was out of step. generate_error_packet() adds a ReadyForQuery to every non-fatal error it sends, whatever its ready argument says.

Fix (be464c2e9): with an implicit Sync pending, both exits build the error without a ReadyForQuery and queue it, as the admin extended-query refusal does. Six older error paths that pass ready=false have the same flaw and are not changed here (pre-existing, also on v3.0): fixing generate_error_packet() itself affects about 29 callers.

Test

pgsql-reg_test_6521_cancel_idle_session_busy_poll-t (legacy-g4), 80 checks:

a cancel of a session idle in a transaction and of an idle fast-forward session: ProxySQL's CPU, read through admin system_cpu, stays low, and the transaction still runs its next query;
in fast forward, a cancelled COPY ... FROM STDIN and two pg_sleep() calls end with 57014, and the COPY inserts nothing;
in libpq and native mode, against a hostgroup with one connection held by another session:
- a cancelled extended statement, INSERT, simple statement and pipeline unit;
- the next statement after a connect timeout and after a refused connection;
- a cancel, timeout or refused connection inside a transaction, which must fail it so COMMIT rolls back;
- Parse/Bind/Execute and then a simple Query, with the wait ending on a timeout and on a refused connection: one ReadyForQuery, after the Query.

Results, on a host ProxySQL with a local PostgreSQL:

origin/v3.0 f02fbf1bb: 18/76 (run before the four implicit-Sync checks were added);
this PR without be464c2e9: 76/80, the four implicit-Sync checks fail;
this PR: 80/80;
CPU after an idle cancel: 100.6% without the fix, 1.0-1.4% with it, with and without --idle-threads.

Also run:

For fix 3: psycopg 3, pgjdbc and node-postgres cancel cases; valgrind, with no new errors or leaks; 500 waiting sessions cancelled at once.
For fix 4: pgsql-native_extq_batch, extq_local_error_in_txn, local_error_txn_state, tx_poisoned_recovery, extended_query_protocol and query_cancel_session_termination_test, with and without be464c2e9. Same results, except one miss: check 10 (pg_terminate_backend) of query_cancel_session_termination_test failed in 1 of 21 runs with the change and 0 of 21 without. That check waits 100 ms for the terminated connection to close, and terminate does not go through the changed code. Root cause not found.

Not run: the Docker CI groups.

Summary by cubic

Fixes query cancels for sessions that are not running a query, which previously spun the worker thread or never reached the backend.

A cancel for a session idle in a transaction set cancel_query on its backend, and only a running query cleared it, so BeforePoll kept using a 0 ms poll timeout and the worker thread busy-polled. The flag is now set only when the session is not WAITING_CLIENT_DATA. Fast-forward sessions get cancel_query again, and the fast-forward handler now sends every cancel to the backend—which ignores it when idle—and resets killed_at so later cancels are sent too; previously a fast-forward cancel never reached the backend, also affecting COPY ... FROM STDIN.

A cancel for a statement still waiting for a backend connection was lost in native mode and often arrived too late in libpq mode, so the statement ran once a connection freed up. handler_again___status_CONNECTING_SERVER() now ends the wait with 57014. The cancel, the connect timeout, and a refused connection share one exit that fails an open transaction, drops the rest of the extended-query unit, and closes a half-open connection; previously those exits left the rest of the unit queued, failing the client's next statement with 26000 and letting a later COMMIT save earlier work. When a simple Query closes the unit, the error is queued without its own ReadyForQuery so the client gets one, after the Query; previously it got two and fell out of step.

Adds regression test pgsql-reg_test_6521_cancel_idle_session_busy_poll-t, which checks ProxySQL's CPU stays low after cancelling idle sessions, that cancels reach the backend in fast forward, that cancels, timeouts, and refused connections inside a transaction fail it so COMMIT rolls back, and that a failed connect wait before an implicit Sync yields exactly one ReadyForQuery.

Summary by CodeRabbit

Bug Fixes
- Cancellation requests no longer interrupt sessions waiting for client data. Requests made while waiting for a backend connection now cancel the waiting statement and clean up its state. Fast-forward sessions handle pending cancellations and can continue processing subsequent queries.
Tests
- Added regression coverage for cancellation in idle and fast-forward sessions, busy-poll behavior, and waits for backend connections. Tests also cover session behavior after cancellation, connection timeouts, and refused connections.