From 432438bb956d10dc3dae2a93ef23d9acf05d7165 Mon Sep 17 00:00:00 2001 From: Rui Zhao Date: Wed, 5 Aug 2026 16:04:05 +0800 Subject: [PATCH 2/2] Add an isolation test for the lock lost when the toaster changes nothing fk-toast-lock covers the case where the toaster really writes something. This covers the other one: the update leaves the external column alone, so the toaster hands its input tuple back unchanged and the new tuple still fits on the same page, even though the buffer lock was dropped just the same. Nothing in that window is blockable from SQL once the toaster has no work to do, so the test drives it with an injection point placed where the buffer lock is released. s1 supplies a lock that is over by the time the update proceeds, which is what leaves heap_update() with checked_lockers set and locker_remains clear. --- src/backend/access/heap/heapam.c | 1 + src/test/modules/injection_points/Makefile | 3 +- .../expected/heap_update_toast_lock.out | 23 ++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/heap_update_toast_lock.spec | 71 +++++++++++++++++++ 5 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/injection_points/expected/heap_update_toast_lock.out create mode 100644 src/test/modules/injection_points/specs/heap_update_toast_lock.spec diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index d68efee985f..34bbe05b8a0 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3929,6 +3929,7 @@ l2: LockBuffer(buffer, BUFFER_LOCK_UNLOCK); buffer_lock_released = true; + INJECTION_POINT("heap_update-toast-unlocked", NULL); /* * Let the toaster do its thing, if needed. diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 25a3ddd890d..9f1a3853b25 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -21,7 +21,8 @@ ISOLATION = basic \ repack_toast \ syscache-update-pruned \ wait_cleanup \ - heap_lock_update + heap_lock_update \ + heap_update_toast_lock # 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/heap_update_toast_lock.out b/src/test/modules/injection_points/expected/heap_update_toast_lock.out new file mode 100644 index 00000000000..bee1ce23bf6 --- /dev/null +++ b/src/test/modules/injection_points/expected/heap_update_toast_lock.out @@ -0,0 +1,23 @@ +Parsed test spec with 4 sessions + +starting permutation: s1_lock s2_update s1_commit s3_insert s1_wake s2_done s4_delete s3_commit +step s1_lock: BEGIN; SELECT id FROM pk_toast WHERE id = 1 FOR NO KEY UPDATE; +id +-- + 1 +(1 row) + +step s2_update: UPDATE pk_toast SET n = n + 1 WHERE id = 1; +step s1_commit: COMMIT; +step s3_insert: BEGIN; INSERT INTO fk_child(pid) VALUES (1); +step s1_wake: + SELECT FROM injection_points_detach('heap_update-toast-unlocked'); + SELECT FROM injection_points_wakeup('heap_update-toast-unlocked'); + +step s2_update: <... completed> +step s1_wake: <... completed> +step s2_done: +step s4_delete: DELETE FROM pk_toast WHERE id = 1; +step s3_commit: COMMIT; +step s4_delete: <... completed> +ERROR: update or delete on table "pk_toast" violates foreign key constraint "fk_child_pid_fkey" on table "fk_child" diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index aaf0536ba7e..a24cdfe0d49 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -53,6 +53,7 @@ tests += { 'syscache-update-pruned', 'wait_cleanup', 'heap_lock_update', + 'heap_update_toast_lock', ], 'runningcheck': false, # see syscache-update-pruned # Some tests wait for all snapshots, so avoid parallel execution diff --git a/src/test/modules/injection_points/specs/heap_update_toast_lock.spec b/src/test/modules/injection_points/specs/heap_update_toast_lock.spec new file mode 100644 index 00000000000..7ad709f02f6 --- /dev/null +++ b/src/test/modules/injection_points/specs/heap_update_toast_lock.spec @@ -0,0 +1,71 @@ +# Tuple lock lost while heap_update() has the buffer unlocked for TOAST +# +# heap_update() marks the old tuple locked and drops the buffer lock while +# it does the TOAST and/or page-extension work. A concurrent foreign key +# check can add a KEY SHARE lock to the tuple during that window, and the +# lock has to survive into the new tuple version. +# +# fk-toast-lock covers the case where the toaster really writes something. +# This one covers the other one: the update does not touch the external +# column, so the toaster hands its input tuple back unchanged and the new +# tuple still fits on the same page, even though the buffer lock was +# dropped just the same. s1 supplies a lock that is over by the time the +# update proceeds, which is what leaves heap_update() with checked_lockers +# set and locker_remains clear. + +setup +{ + CREATE EXTENSION injection_points; + + CREATE TABLE pk_toast ( + id int PRIMARY KEY, + n int, + payload text + ); + ALTER TABLE pk_toast ALTER COLUMN payload SET STORAGE EXTERNAL; + + CREATE TABLE fk_child ( + cid serial PRIMARY KEY, + pid int REFERENCES pk_toast(id) + ); + + INSERT INTO pk_toast VALUES (1, 0, repeat('x', 10000)); +} + +teardown +{ + DROP TABLE fk_child, pk_toast; + DROP EXTENSION injection_points; +} + +# A lock that is released before the update gets to the TOAST step. +session s1 +step s1_lock { BEGIN; SELECT id FROM pk_toast WHERE id = 1 FOR NO KEY UPDATE; } +step s1_commit { COMMIT; } +step s1_wake { + SELECT FROM injection_points_detach('heap_update-toast-unlocked'); + SELECT FROM injection_points_wakeup('heap_update-toast-unlocked'); +} + +# Non-key UPDATE that leaves the external column alone. +session s2 +setup { + SELECT FROM injection_points_set_local(); + SELECT FROM injection_points_attach('heap_update-toast-unlocked', 'wait'); +} +step s2_update { UPDATE pk_toast SET n = n + 1 WHERE id = 1; } +# Only runs once s2_update has finished, so that s4_delete below reads the +# new tuple version rather than blocking on the old one. +step s2_done { } + +# Insert a child row; the foreign key check takes KEY SHARE on the parent. +session s3 +step s3_insert { BEGIN; INSERT INTO fk_child(pid) VALUES (1); } +step s3_commit { COMMIT; } + +# Delete the parent row, which must wait for s3. +session s4 +step s4_delete { DELETE FROM pk_toast WHERE id = 1; } + +permutation s1_lock s2_update s1_commit s3_insert s1_wake(s2_update) + s2_done s4_delete s3_commit -- 2.43.7