diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 36b0a9a4e75..5366303e02f 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3621,14 +3621,25 @@ l2: else update_xact = InvalidTransactionId; - /* - * There was no UPDATE in the MultiXact; or it aborted. No - * TransactionIdIsInProgress() call needed here, since we called - * MultiXactIdWait() above. - */ - if (!TransactionIdIsValid(update_xact) || - TransactionIdDidAbort(update_xact)) + if (!TransactionIdIsValid(update_xact)) + { + /* no UPDATE in the MultiXact */ can_continue = true; + } + else + { + /* + * There was another UPDATE. Did it abort? + * + * Note: We check !TransactionIdDidCommit() instead of + * TransactionIdDidAbort(), to treat crashed transactions as + * aborted. No TransactionIdIsInProgress() call is needed + * here, because the MultiXactIdWait() call above already + * waited for it to finish. + */ + if (!TransactionIdDidCommit(update_xact)) + can_continue = true; + } } else if (TransactionIdIsCurrentTransactionId(xwait)) { @@ -7862,8 +7873,9 @@ DoesMultiXactIdConflict(MultiXactId multi, uint16 infomask, if (ISUPDATE_from_mxstatus(members[i].status)) { - /* ignore aborted updaters */ - if (TransactionIdDidAbort(memxid)) + /* ignore aborted (or crashed) updaters */ + if (!TransactionIdIsInProgress(memxid) && + !TransactionIdDidCommit(memxid)) continue; } else diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 75bf4e9b3d9..92319290cf7 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -1926,6 +1926,12 @@ RecordTransactionAbort(bool isSubXact) if (ndroppedstats) pfree(droppedstats); + /* + * Test the window where the transaction is aborted in pg_xact but still + * present in ProcArray. + */ + INJECTION_POINT("transaction-abort-after-clog", NULL); + return latestXid; } diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 2463d6e0fb9..775ad37860d 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -35,7 +35,8 @@ ISOLATION = basic \ syscache-update-pruned \ $(WAIT_CLEANUP_TST) \ heap_lock_update \ - on_conflict_probe_window + on_conflict_probe_window \ + multixact-aborted-updater # some isolation tests require wal_level=replica ISOLATION_OPTS = --temp-config $(top_srcdir)/src/test/modules/injection_points/extra.conf diff --git a/src/test/modules/injection_points/expected/multixact-aborted-updater.out b/src/test/modules/injection_points/expected/multixact-aborted-updater.out new file mode 100644 index 00000000000..40750714734 --- /dev/null +++ b/src/test/modules/injection_points/expected/multixact-aborted-updater.out @@ -0,0 +1,84 @@ +Parsed test spec with 4 sessions + +starting permutation: s1begin s1update s2lock s1abort s3nowait wake +step s1begin: BEGIN; +step s1update: UPDATE mxact_abort SET filler = 's1' WHERE id = 1; +step s2lock: SELECT * FROM mxact_abort WHERE id = 1 FOR KEY SHARE; +id|filler +--+------- + 1|initial +(1 row) + +step s1abort: ROLLBACK; +step s3nowait: SELECT * FROM mxact_abort WHERE id = 1 FOR NO KEY UPDATE NOWAIT; +ERROR: could not obtain lock on row in relation "mxact_abort" +step wake: + SELECT FROM injection_points_detach('transaction-abort-after-clog'); + SELECT FROM injection_points_wakeup('transaction-abort-after-clog'); + +step s1abort: <... completed> + +starting permutation: s1begin s1update s2lock s1abort s3update wake +step s1begin: BEGIN; +step s1update: UPDATE mxact_abort SET filler = 's1' WHERE id = 1; +step s2lock: SELECT * FROM mxact_abort WHERE id = 1 FOR KEY SHARE; +id|filler +--+------- + 1|initial +(1 row) + +step s1abort: ROLLBACK; +step s3update: UPDATE mxact_abort SET filler = 's3' WHERE id = 1; +step wake: + SELECT FROM injection_points_detach('transaction-abort-after-clog'); + SELECT FROM injection_points_wakeup('transaction-abort-after-clog'); + +step s3update: <... completed> +step s1abort: <... completed> + +starting permutation: s1begin s1update s2lock s1abort s3delete wake s3check +step s1begin: BEGIN; +step s1update: UPDATE mxact_abort SET filler = 's1' WHERE id = 1; +step s2lock: SELECT * FROM mxact_abort WHERE id = 1 FOR KEY SHARE; +id|filler +--+------- + 1|initial +(1 row) + +step s1abort: ROLLBACK; +step s3delete: DELETE FROM mxact_abort WHERE id = 1; +step wake: + SELECT FROM injection_points_detach('transaction-abort-after-clog'); + SELECT FROM injection_points_wakeup('transaction-abort-after-clog'); + +step s3delete: <... completed> +step s1abort: <... completed> +step s3check: SELECT * FROM mxact_abort ORDER BY id; +id|filler +--+------ +(0 rows) + + +starting permutation: s1begin s1update s2lock s1abort s3keyupdate wake s3check +step s1begin: BEGIN; +step s1update: UPDATE mxact_abort SET filler = 's1' WHERE id = 1; +step s2lock: SELECT * FROM mxact_abort WHERE id = 1 FOR KEY SHARE; +id|filler +--+------- + 1|initial +(1 row) + +step s1abort: ROLLBACK; +step s3keyupdate: UPDATE mxact_abort SET id = 2 WHERE id = 1; +step wake: + SELECT FROM injection_points_detach('transaction-abort-after-clog'); + SELECT FROM injection_points_wakeup('transaction-abort-after-clog'); + +step s3keyupdate: <... completed> +step s1abort: <... completed> +step s3check: SELECT * FROM mxact_abort ORDER BY id; +id|filler +--+------- + 2|initial +(1 row) + diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index b4a0079484b..60148f96fda 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -43,6 +43,7 @@ injection_points_isolation = [ 'syscache-update-pruned', 'heap_lock_update', 'on_conflict_probe_window', + 'multixact-aborted-updater', ] # wait_cleanup terminates another backend, whose FATAL message can be lost diff --git a/src/test/modules/injection_points/specs/multixact-aborted-updater.spec b/src/test/modules/injection_points/specs/multixact-aborted-updater.spec new file mode 100644 index 00000000000..98d265a2e94 --- /dev/null +++ b/src/test/modules/injection_points/specs/multixact-aborted-updater.spec @@ -0,0 +1,107 @@ +# Test concurrent behavior of transaction abort in the window between marking +# the transaction as aborted in pg_xact and removing it from the ProcArray. +# +# In most cases, the correct way to check if a transaction aborted is to first +# check TransactionIdIsInProgress(), followed by !TransactionDidCommit(). The +# TransactionIdIsInProgress() call ensures that the transaction is only +# considered as aborted after it's been removed from the ProcArray, and using +# !TransactionDidCommit() instead of TransactionIdDidAbort() ensures that you +# treat crashed transactions -- i.e. transactions that were in progress when the +# system crashed and didn't write an ABORT WAL record -- as aborted too. +# +# This test uses an injection point to pause an aborting transaction between +# updating pg_xact and removing it from the ProcArray, and performs different +# concurrent operations involving multixids. This exercises various codepaths +# where the correct ordering of TransactionIdIsInProgress() and +# TransactionIdDidCommit() matters. + +setup +{ + CREATE EXTENSION injection_points; + + CREATE TABLE mxact_abort (id int PRIMARY KEY, filler text); + INSERT INTO mxact_abort VALUES (1, 'initial'); +} + +teardown +{ + DROP TABLE mxact_abort; + DROP EXTENSION injection_points; +} + +# Transaction 1 performs an UPDATE, aborts, and pauses between updating pg_xact +# and ProcArray cleanup +session s1 +setup { + SELECT FROM injection_points_set_local(); + SELECT FROM injection_points_attach('transaction-abort-after-clog', 'wait'); +} +step s1begin { BEGIN; } +step s1update { UPDATE mxact_abort SET filler = 's1' WHERE id = 1; } +step s1abort { ROLLBACK; } + +# Transaction 2 locks the row in KEY SHARE mode, to cause a multixact to be +# created. +session s2 +step s2lock { SELECT * FROM mxact_abort WHERE id = 1 FOR KEY SHARE; } + +# Transaction 3 performs a concurrent operation while transaction 1 is aborting +session s3 +step s3nowait { SELECT * FROM mxact_abort WHERE id = 1 FOR NO KEY UPDATE NOWAIT; } +step s3update { UPDATE mxact_abort SET filler = 's3' WHERE id = 1; } +step s3delete { DELETE FROM mxact_abort WHERE id = 1; } +step s3keyupdate { UPDATE mxact_abort SET id = 2 WHERE id = 1; } +step s3check { SELECT * FROM mxact_abort ORDER BY id; } + +session waker +step wake { + SELECT FROM injection_points_detach('transaction-abort-after-clog'); + SELECT FROM injection_points_wakeup('transaction-abort-after-clog'); +} + +# A SELECT .. NOWAIT while the updating transaction is paused in the abort +# considers the updater as still running, and reports an error. +permutation + s1begin + s1update + s2lock # succeeds and creates a multixid + s1abort # blocks on the injection point + # fails, the aborting updater is still considered running + s3nowait + wake + +# A conflicting UPDATE waits for the first updater to fully finish. +# +# Report the abort after the conflicting operation in these permutations +# to keep the completion output stable. +permutation + s1begin + s1update + s2lock + s1abort(s3update) + # blocks waiting for the aborting updater to finish + s3update + wake + +# Same for a DELETE +permutation + s1begin + s1update + s2lock + s1abort(s3delete) + # blocks waiting for the aborting updater to finish + s3delete + wake + s3check + +# A key-changing UPDATE must also wait before deciding which members of +# the multixact survive. +permutation + s1begin + s1update + s2lock + s1abort(s3keyupdate) + # blocks waiting for the aborting updater to finish + s3keyupdate + wake + s3check diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index 7982bb6b0ba..87dbdb6326e 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -67,6 +67,7 @@ tests += { 't/056_standby_snapshot_export.pl', 't/057_snapshot_commit_race.pl', 't/059_remote_apply_status_interval.pl', + 't/060_multixact_crashed_updater.pl', ], }, } diff --git a/src/test/recovery/t/060_multixact_crashed_updater.pl b/src/test/recovery/t/060_multixact_crashed_updater.pl new file mode 100644 index 00000000000..85f25722fd1 --- /dev/null +++ b/src/test/recovery/t/060_multixact_crashed_updater.pl @@ -0,0 +1,75 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test that a crashed updating transaction that is part of a multixid is treated +# as aborted. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; +$node->append_conf('postgresql.conf', 'max_prepared_transactions = 10'); +$node->start; + +$node->safe_psql( + 'postgres', q( + CREATE TABLE mx_crashed (id int PRIMARY KEY, payload text); + INSERT INTO mx_crashed VALUES (1, 'initial'); +)); + +# Keep the updater open while another transaction locks the same row. This +# creates a multixid with the updating and locking xids as members. +my $updater = $node->background_psql('postgres'); +$updater->query_safe('BEGIN'); +$updater->query_safe( + q(UPDATE mx_crashed SET payload = 'crashed' WHERE id = 1)); + +$node->safe_psql( + 'postgres', q( + BEGIN; + SELECT id FROM mx_crashed WHERE id = 1 FOR KEY SHARE; + PREPARE TRANSACTION 'mx_crashed_locker'; +)); + +# The PREPARE TRANSACTION flushed the preceding heap update and multixact WAL +# records, too. Kill the server to leave the updating transaction as crashed, +# ie. it is marked neither as committed nor aborted in pg_xact. +$node->stop('immediate'); +$updater->{run}->finish; +$node->start; + +# Update the row again after restart. The crashed updater should be ignored, but +# the FOR KEY SHARE lock should remain. +my ($stdout, $stderr); +my $ret = $node->psql( + 'postgres', q( + BEGIN ISOLATION LEVEL REPEATABLE READ; + UPDATE mx_crashed SET payload = 'after crash' WHERE id = 1; + COMMIT; +), + stdout => \$stdout, + stderr => \$stderr); +is($ret, 0, 'repeatable read update ignores the crashed updater') + or diag($stderr); + +is( $node->safe_psql( + 'postgres', q(SELECT payload FROM mx_crashed WHERE id = 1)), + 'after crash', + 'row was updated'); + +# Check that the row remains locked for the locking transaction +$node->psql( + 'postgres', + 'SELECT * FROM mx_crashed FOR UPDATE NOWAIT', + stderr => \$stderr); +like( + $stderr, + qr/could not obtain lock on row in relation/, + "prepared share locker survives after crash"); + +$node->stop(); +done_testing();