Problem/Motivation
The database driver for PostgreSQL is now mimicking how the database driver
for MySQL/MariaDB does things. To make the database driver for PostgreSQL work
like the one for MySQL/MariaDB it adds a lot of savepoints in transactions.
Savepoints in transactions are not free. Starting and ending a savepoint are
both a round trip to the database. Every Insert, Update, Upsert and Select
query uses savepoints. Instead of a single round trip to the database, we
have 3 round trips to the database. There is some performance improvement
possible.
Why the savepoints are there
PostgreSQL: any error poisons the whole transaction. When a
statement fails inside a transaction — even an "expected" failure like a
duplicate-key violation or a query against a table that doesn't exist yet —
PostgreSQL marks the entire transaction as aborted. Every subsequent statement
is rejected with
ERROR: current transaction is aborted, commands ignored
until end of transaction block until you ROLLBACK. You
can't just catch the exception in PHP and carry on; the transaction is dead.
The only way to recover part of a transaction is a savepoint:
SAVEPOINT s1 before the risky statement, then
ROLLBACK TO SAVEPOINT s1 if it fails (which restores the
transaction to a usable state) or RELEASE SAVEPOINT s1 if it
succeeds.
MySQL/InnoDB: only the statement fails. When a statement
errors, InnoDB rolls back just that statement (or in a few edge cases like
deadlock, the transaction — but it tells you). The transaction itself remains
open and usable, so a catch block in PHP can simply try something else and
continue. No savepoint needed.
Why this matters for Drupal specifically: Drupal core has
several patterns that deliberately race or probe:
- Merge / upsert-style logic: try an
INSERT, catch the
integrity-constraint violation, fall back toUPDATE. - Cache, lock, flood, key-value, queue backends: write to a table, and if
it fails with "table not found", lazily create the table and retry
(ensureTableExists()pattern). ExceptionHandlerfor inserts catching duplicate-key
exceptions.
On MySQL these catch-and-retry patterns just work, even when the caller has
an outer transaction open (e.g. during entity save). On PostgreSQL, without a
savepoint around the risky statement, the caller's transaction would be
aborted as collateral damage — an entity save would blow up because a
cache-set inside it hit a duplicate key. That's why the pgsql driver
historically wraps such statements in savepoints (addSavepoint()
/ releaseSavepoint() / rollbackSavepoint() on the
connection), and why this work moves those savepoints out of blanket driver
wrapping and into the specific consumers that actually do catch-and-continue —
the savepoint round-trips cost extra network chatter and each savepoint has
server-side overhead, so you only want them where a failure is genuinely
expected and recoverable.
Proposed resolution
- All savepoints in the Insert, Select, Update, and Upsert queries are
removed. - The backend database storage classes that use the trick with ensure
table exists have been replaced with a verify that the table exists.
Performance testing
Base queries
| Operation | baseline | improved | Speedup |
|---|---|---|---|
| select | 125.4 | 52.0 | 2.41x |
| insert | 124.7 | 52.6 | 2.37x |
| update | 125.0 | 53.4 | 2.34x |
| upsert (row exists) | 127.8 | 56.8 | 2.25x |
| merge (row exists) | 261.3 | 109.7 | 2.38x |
| merge (row missing) | 257.2 | 182.9 | 1.41x |
Backend storage classes
| Suite | Ops | Speedup |
|---|---|---|
| keyvalue | set / get / has / delete | 2.1-2.5x |
| keyvalue | setIfNotExists (merge, insert path) | 1.4x |
| kv_expirable | setWithExpire / get | 2.3x |
| config | write / read / exists | 2.3-2.5x |
| queue | createItem / claimItem / deleteItem | 2.0-2.5x |
| flood | register / isAllowed / clear | 2.1-2.4x |
| lock | acquire fresh / extend / maybeAvailable / release | 1.9-2.5x |
| lock | acquire contended | 6.2x |
| batch | getId / create / load / delete | 2.2-2.6x |
| session | write / read / destroy | 2.0-2.5x |
| menu | rebuild 200 links | 1.8x (192ms → 104ms) |
| router | dump 200 routes | 1.07x |
I have tested the baseline to the PR and the overall difference is about 2x
to 2.5x. performance improvement.
Remaining tasks
To make that possible we need a fundamental change in how the database
driver for PostgreSQL works. Does the database driver keep mimicking the
database driver for MySQL/MariaDB or are we going to change it and optimize
the database driver for working with a PostgreSQL database? That decision is
for the Drupal Core backend framework managers and maybe even the release
managers.
For the committer
The changes to the .gitlab-ci.yml file need to be removed before merging!
| Comment | File | Size | Author |
|---|
Issue fork drupal-3615690
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
daffie commentedComment #4
daffie commentedThe CI pipeline is green for MySQL and PostgreSQL on PHP 8.5.
Ready for a review.
Comment #5
daffie commentedThe override for merge queries will be replaced when #3315265: Improve support of native MERGE with RETURNING merge_action() lands.
Comment #6
daffie commentedThe savepoint in the method Schema::queryTableInformation() will be removed in #3615649: Cache the PostgreSQL table information.
The added methods ::verifyTableInTransaction() and verifyTable() should use the caching service from the same issue.
Comment #7
daffie commentedDisclosure: I have used AI on the PR, the IS and the CR.
Comment #8
daffie commentedAnother example for why we Drupal on PostgreSQL should not be mimicking Drupal on MySQL/MariaDB is #3359406: Postgres: Sorting NULL values causes performance degradation.
Comment #9
daffie commentedYet another example for why we Drupal on PostgreSQL should not be mimicking Drupal on MySQL/MariaDB is #3361618: Postgres forcing cases case-insensitivity causes serious performance degradation
Comment #10
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #11
daffie commentedThe CI pipeline is green again.
Comment #12
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #13
daffie commentedRebased the PR. Back to NR.