>> There would no the bug. There would no this discussion. >> If only array bounds were checked with asserts.
> Yeah, more asserts == good, usually. Maybe. My thoughts were that If the MaxOldestSlot is set correctly at initialization time, and we guarantee that by asserting its size against the total number of procs, that should be sufficient, and we don't need to create wrapper functions or add assertions on every access. But. I don't have a strong opinion either way. Anyhow, it looks like the proposals now are aiming towards eliminating the need to account for auxiliary procs, and that is probably OK. It's a small bit of memory saved, but slightly bit more code. > With extremely low configured MaxBackends it will overflow to > BufferDescriptors array. Certainly only test configurations may use such > tiny settings. > > I found the bug because I increased NUM_AUXILIARY_PROCS by bunch of other > specialized processes, so overflow to BufferDescriptors happened even with > parameters already set in tests. It's unfortunately a bit worse. Here is a repro that shows 2 prepared transactions being created after a shared lock is taken on a table with 1 row. A subsequent delete is able to complete, where we would expect it to be blocked until the prepared transactions COMMIT or ROLLBACK. With simply adding NUM_AUXILIARY_PROCS to MaxOldestSlot, it works as expected, and the delete is blocked. ``` -#define MaxOldestSlot (MaxBackends + max_prepared_xacts) +#define MaxOldestSlot (MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts) ``` I also tried with Heikki's proposal and the test succeeded. I did not review the patch thoroughly yet, but I think this test case should be added. The test does require 2 prepared transactions to exercise MultiXactIdSetOldestVisible(), which then results in reading garbage values from the array and determining incorrect visibility of the row. -- Sami Imseih Amazon Web Services (AWS)
From 0d56ffa57803b5059bd40589f659840f80d29adc Mon Sep 17 00:00:00 2001 From: Sami Imseih <[email protected]> Date: Fri, 27 Feb 2026 10:35:41 -0600 Subject: [PATCH v1 1/1] repro --- .../isolation/expected/multixact-aux-proc.out | 24 +++++++++++++++ .../isolation/expected/multixact-prep-tx.out | 30 +++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/multixact-aux-proc.spec | 29 ++++++++++++++++++ .../isolation/specs/multixact-prep-tx.spec | 29 ++++++++++++++++++ 5 files changed, 113 insertions(+) create mode 100644 src/test/isolation/expected/multixact-aux-proc.out create mode 100644 src/test/isolation/expected/multixact-prep-tx.out create mode 100644 src/test/isolation/specs/multixact-aux-proc.spec create mode 100644 src/test/isolation/specs/multixact-prep-tx.spec diff --git a/src/test/isolation/expected/multixact-aux-proc.out b/src/test/isolation/expected/multixact-aux-proc.out new file mode 100644 index 00000000000..08bba739bc9 --- /dev/null +++ b/src/test/isolation/expected/multixact-aux-proc.out @@ -0,0 +1,24 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_begin_p1 s1_share_p1 s1_prep_p1 s1_begin_p2 s1_share_p2 s1_prep_p2 s2_lock_timeout s2_delete s2_rollback_prepared_p1 s2_rollback_prepared_p2 +step s1_begin_p1: BEGIN; +step s1_share_p1: SELECT * FROM dont_delete WHERE id = 1 FOR SHARE; +id|data +--+--------- + 1|test data +(1 row) + +step s1_prep_p1: PREPARE TRANSACTION 'p1'; +step s1_begin_p2: BEGIN; +step s1_share_p2: SELECT * FROM dont_delete WHERE id = 1 FOR SHARE; +id|data +--+--------- + 1|test data +(1 row) + +step s1_prep_p2: PREPARE TRANSACTION 'p2'; +step s2_lock_timeout: SET lock_timeout TO '2s'; +step s2_delete: DELETE FROM dont_delete WHERE id = 1; +ERROR: canceling statement due to lock timeout +step s2_rollback_prepared_p1: ROLLBACK PREPARED 'p1'; +step s2_rollback_prepared_p2: ROLLBACK PREPARED 'p2'; diff --git a/src/test/isolation/expected/multixact-prep-tx.out b/src/test/isolation/expected/multixact-prep-tx.out new file mode 100644 index 00000000000..68470cf33d9 --- /dev/null +++ b/src/test/isolation/expected/multixact-prep-tx.out @@ -0,0 +1,30 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_begin_p1 s1_share_p1 s1_prep_p1 s1_begin_p2 s1_share_p2 s1_prep_p2 s2_lock_timeout s2_delete s2_select s2_rollback_prepared_p1 s2_rollback_prepared_p2 +step s1_begin_p1: BEGIN; +step s1_share_p1: SELECT * FROM block_delete WHERE id = 1 FOR SHARE; +id|data +--+--------- + 1|test data +(1 row) + +step s1_prep_p1: PREPARE TRANSACTION 'p1'; +step s1_begin_p2: BEGIN; +step s1_share_p2: SELECT * FROM block_delete WHERE id = 1 FOR SHARE; +id|data +--+--------- + 1|test data +(1 row) + +step s1_prep_p2: PREPARE TRANSACTION 'p2'; +step s2_lock_timeout: SET lock_timeout TO '2s'; +step s2_delete: DELETE FROM block_delete WHERE id = 1; +ERROR: canceling statement due to lock timeout +step s2_select: SELECT * FROM block_delete WHERE id = 1; +id|data +--+--------- + 1|test data +(1 row) + +step s2_rollback_prepared_p1: ROLLBACK PREPARED 'p1'; +step s2_rollback_prepared_p2: ROLLBACK PREPARED 'p2'; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 4e466580cd4..ff7f93ea6ef 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -66,6 +66,7 @@ test: aborted-keyrevoke test: multixact-no-deadlock test: multixact-no-forget test: multixact-stats +test: multixact-prep-tx test: lock-committed-update test: lock-committed-keyupdate test: update-locked-tuple diff --git a/src/test/isolation/specs/multixact-aux-proc.spec b/src/test/isolation/specs/multixact-aux-proc.spec new file mode 100644 index 00000000000..2cddf53d1d1 --- /dev/null +++ b/src/test/isolation/specs/multixact-aux-proc.spec @@ -0,0 +1,29 @@ +# Test that dummy processes for prepared transactions are properly accounted for in multixact tracking + +setup +{ + CREATE TABLE block_delete (id int PRIMARY KEY, data text); + INSERT INTO block_delete VALUES (1, 'test data'); +} + +teardown +{ + DROP TABLE block_delete; +} + +session s1 +step s1_begin_p1 { BEGIN; } +step s1_share_p1 { SELECT * FROM block_delete WHERE id = 1 FOR SHARE; } +step s1_prep_p1 { PREPARE TRANSACTION 'p1'; } +step s1_begin_p2 { BEGIN; } +step s1_share_p2 { SELECT * FROM block_delete WHERE id = 1 FOR SHARE; } +step s1_prep_p2 { PREPARE TRANSACTION 'p2'; } + +session s2 +step s2_lock_timeout { SET lock_timeout TO '2s'; } +step s2_delete { DELETE FROM block_delete WHERE id = 1; } +step s2_select { SELECT * FROM block_delete WHERE id = 1; } +step s2_rollback_prepared_p1 { ROLLBACK PREPARED 'p1'; } +step s2_rollback_prepared_p2 { ROLLBACK PREPARED 'p2'; } + +permutation s1_begin_p1 s1_share_p1 s1_prep_p1 s1_begin_p2 s1_share_p2 s1_prep_p2 s2_lock_timeout s2_delete s2_select s2_rollback_prepared_p1 s2_rollback_prepared_p2 diff --git a/src/test/isolation/specs/multixact-prep-tx.spec b/src/test/isolation/specs/multixact-prep-tx.spec new file mode 100644 index 00000000000..2cddf53d1d1 --- /dev/null +++ b/src/test/isolation/specs/multixact-prep-tx.spec @@ -0,0 +1,29 @@ +# Test that dummy processes for prepared transactions are properly accounted for in multixact tracking + +setup +{ + CREATE TABLE block_delete (id int PRIMARY KEY, data text); + INSERT INTO block_delete VALUES (1, 'test data'); +} + +teardown +{ + DROP TABLE block_delete; +} + +session s1 +step s1_begin_p1 { BEGIN; } +step s1_share_p1 { SELECT * FROM block_delete WHERE id = 1 FOR SHARE; } +step s1_prep_p1 { PREPARE TRANSACTION 'p1'; } +step s1_begin_p2 { BEGIN; } +step s1_share_p2 { SELECT * FROM block_delete WHERE id = 1 FOR SHARE; } +step s1_prep_p2 { PREPARE TRANSACTION 'p2'; } + +session s2 +step s2_lock_timeout { SET lock_timeout TO '2s'; } +step s2_delete { DELETE FROM block_delete WHERE id = 1; } +step s2_select { SELECT * FROM block_delete WHERE id = 1; } +step s2_rollback_prepared_p1 { ROLLBACK PREPARED 'p1'; } +step s2_rollback_prepared_p2 { ROLLBACK PREPARED 'p2'; } + +permutation s1_begin_p1 s1_share_p1 s1_prep_p1 s1_begin_p2 s1_share_p2 s1_prep_p2 s2_lock_timeout s2_delete s2_select s2_rollback_prepared_p1 s2_rollback_prepared_p2 -- 2.50.1 (Apple Git-155)
