Skip to content

Tolerate deallocation of statements the server has already dropped - #74

Open
webpatser wants to merge 1 commit into
amphp:2.xfrom
webpatser:tolerate-deallocate-of-dropped-statements
Open

Tolerate deallocation of statements the server has already dropped#74
webpatser wants to merge 1 commit into
amphp:2.xfrom
webpatser:tolerate-deallocate-of-dropped-statements

Conversation

@webpatser

Copy link
Copy Markdown

Follow-up to amphp/sql-common#8. Deallocating a prepared statement can race the pool's DISCARD ALL connection reset: DISCARD ALL drops every server-side prepared statement, so the queued DEALLOCATE for a statement closed just before the connection was reset fails with prepared statement "..." does not exist.

Two problems fall out of that on the pgsql backend:

  1. The DEALLOCATE ran through query(), so its error result went through createResult(). The fatal-error branch there drains the connection (while (pg_connection_busy && pg_get_result)), which can consume responses belonging to an operation dispatched after the deallocation. That operation then awaits a reply that never comes, wedging the connection permanently. With sql-common#8 in place (declined statements are closed), this shape reproduces deterministically:
$pool = new PostgresConnectionPool(PostgresConfig::fromString('...'), 1);

$statement = $pool->prepare('SELECT 1');
$result = $statement->execute();
iterator_to_array($result);

// $result stays referenced; the declined statement is closed, queueing DEALLOCATE.
// The next checkout runs DISCARD ALL, the DEALLOCATE fails, and its error drain
// eats the response of the execute below, which never completes.
$second = $pool->prepare('SELECT 2');
iterator_to_array($second->execute());
  1. On the pq backend the failure was not drained but still errored an unobserved future.

Fix: send the DEALLOCATE without routing it through result processing and tolerate failure on both backends; the statement being gone is the desired end state either way. The statement cache entry is now removed regardless of the outcome.

The new StatementDeallocateTest exercises deallocation racing DISCARD ALL on a single-connection pool; the deterministic wedge above needs sql-common#8 to trigger, so it is documented here rather than encoded as a test that would fail against the current sql-common release.

Test run: 121 PgSql tests + the new test green against PostgreSQL 16 (pq suite skipped, ext-pq unavailable locally).

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant