From e0ec78b896ddae6be382371e2132dc1bb9cfe08c Mon Sep 17 00:00:00 2001 From: Shihao Date: Thu, 24 Sep 2026 23:46:58 -0400 Subject: [PATCH v4 1/2] Test TOAST rewrite during REPACK (CONCURRENTLY) startup Add a permutation to repack_toast.spec that tries to rewrite the TOAST relation while the decoding worker waits for a running transaction. The rewrite has to fail on lock_timeout, and REPACK has to keep all the concurrent changes. Without the fix, the rewrite succeeds and the concurrent updates of TOASTed columns are lost. Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com --- .../expected/repack_toast.out | 151 +++++++++++++++++- .../injection_points/specs/repack_toast.spec | 51 ++++++ 2 files changed, 201 insertions(+), 1 deletion(-) diff --git a/src/test/modules/injection_points/expected/repack_toast.out b/src/test/modules/injection_points/expected/repack_toast.out index 95e7b19893e..756e7e8187f 100644 --- a/src/test/modules/injection_points/expected/repack_toast.out +++ b/src/test/modules/injection_points/expected/repack_toast.out @@ -1,4 +1,4 @@ -Parsed test spec with 2 sessions +Parsed test spec with 3 sessions starting permutation: s1_wait_before_lock s2_updates s2_check s2_wakeup_before_lock s1_check injection_points_attach @@ -124,3 +124,152 @@ injection_points_detach (1 row) + +starting permutation: s2_begin s1_wait_before_lock s3_rewrite_toast s3_noop s2_commit s2_updates s2_check s2_wakeup_before_lock s1_check +injection_points_attach +----------------------- + +(1 row) + +step s2_begin: + BEGIN; + SELECT pg_current_xact_id() IS NOT NULL AS has_xid; + +has_xid +------- +t +(1 row) + +step s1_wait_before_lock: + REPACK (CONCURRENTLY) repack_toast; + +step s3_rewrite_toast: + DO $$ + BEGIN + EXECUTE format('REPACK %s', + (SELECT reltoastrelid::regclass FROM pg_class + WHERE relname = 'repack_toast')); + END; + $$; + +step s3_rewrite_toast: <... completed> +ERROR: canceling statement due to lock timeout +step s3_noop: +step s2_commit: + COMMIT; + +step s2_updates: + DELETE FROM repack_toast WHERE i=1; + INSERT INTO repack_toast(i, j, k) VALUES (1, gen_external(), gen_compressible(1)); + + -- existing toast data unchanged. (This covers the case where we + -- adjust the toast pointer.) + UPDATE repack_toast SET i=i+300 where i % 10 = 2 RETURNING OLD.i, NEW.i; + + -- "j" is here an external indirect, written to the file separately. + UPDATE repack_toast SET j=gen_external() where i % 10 = 3 RETURNING OLD.i, NEW.i; + + -- the updated value of "j" is compressed. + UPDATE repack_toast SET j=gen_compressible(1), k=k||'' where i % 10 = 4 RETURNING i; + + -- the updated value of "j" is compressed externally. + UPDATE repack_toast SET j=gen_compressible_external(2) where i % 10 = 5 RETURNING i; + + -- the updated value of "j" stays inline. + UPDATE repack_toast SET j=gen_inline(), k=repeat(k,5) where i % 10 = 6 RETURNING i; + + -- updated value of "j" is a short varlena; "k" is written separately. + UPDATE repack_toast SET j=gen_short(), k=gen_external() where i % 10 = 7 RETURNING i; + + i| i +--+--- + 2|302 +12|312 +(2 rows) + + i| i +--+-- + 3| 3 +13|13 +(2 rows) + + i +-- + 4 +14 +(2 rows) + + i +-- + 5 +15 +(2 rows) + + i +-- + 6 +16 +(2 rows) + + i +-- + 7 +17 +(2 rows) + +step s2_check: + INSERT INTO relfilenodes(node) + SELECT c2.relfilenode + FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid + WHERE c1.relname='repack_toast'; + + INSERT INTO data_s2(i, j, j_toast, k, k_toast) + SELECT i, j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast, + k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast + FROM repack_toast; + +step s2_wakeup_before_lock: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s1_wait_before_lock: <... completed> +step s1_check: + INSERT INTO relfilenodes(node) + SELECT c2.relfilenode + FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid + WHERE c1.relname='repack_toast'; + + SELECT count(DISTINCT node) FROM relfilenodes; + + INSERT INTO data_s1(i, j, j_toast, k, k_toast) + SELECT i, + j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast, + k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast + FROM repack_toast; + + -- this should be empty + SELECT d1.i, substring(d1.j FOR 12) AS d1_j, substring(d1.k FOR 12) AS d1_k, + d2.i, substring(d2.j FOR 12) AS d2_j, substring(d2.k FOR 12) AS d2_k, + d1.j_toast as d1_j_tst, d2.j_toast as d2_j_tst, + d1.k_toast as d1_k_tst, d2.k_toast AS d2_k_tst + FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j, k) + WHERE d1.i ISNULL OR d2.i ISNULL; + +count +----- + 4 +(1 row) + +i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst +-+----+----+-+----+----+--------+--------+--------+-------- +(0 rows) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/specs/repack_toast.spec b/src/test/modules/injection_points/specs/repack_toast.spec index cc8f034d016..a105a44848e 100644 --- a/src/test/modules/injection_points/specs/repack_toast.spec +++ b/src/test/modules/injection_points/specs/repack_toast.spec @@ -125,6 +125,18 @@ teardown session s2 +# Keep a transaction with XID open, so that the decoding worker has to wait +# before it can build the initial snapshot. +step s2_begin +{ + BEGIN; + SELECT pg_current_xact_id() IS NOT NULL AS has_xid; +} +step s2_commit +{ + COMMIT; +} + # Test different kinds of toast data changes. step s2_updates { @@ -170,6 +182,32 @@ step s2_wakeup_before_lock SELECT injection_points_wakeup('repack-concurrently-before-lock'); } +# Try to rewrite the TOAST relation. The decoding worker only decodes the +# changes of the TOAST relation stored under the relfilenumber it saw when +# starting, so REPACK must not let the TOAST relation be rewritten after that. +# Otherwise the TOAST chunks of the concurrent changes are not decoded, and the +# changes are lost. +# +# The name of the TOAST relation is only known at run time, hence the DO +# block, and REPACK rather than VACUUM FULL, which cannot run in one. +# +# Don't wait for the lock. The rewrite gets an XID before it waits, and once +# s2 commits, the decoding worker would wait for that XID, which is a deadlock. +session s3 +setup { SET lock_timeout = 10; } +step s3_rewrite_toast +{ + DO $$ + BEGIN + EXECUTE format('REPACK %s', + (SELECT reltoastrelid::regclass FROM pg_class + WHERE relname = 'repack_toast')); + END; + $$; +} +# Empty step, so that s2 cannot go on until s3_rewrite_toast is done. +step s3_noop { } + # Test if data changes introduced while one session is performing REPACK # CONCURRENTLY find their way into the table. permutation @@ -178,3 +216,16 @@ permutation s2_check s2_wakeup_before_lock s1_check + +# Same, but try to rewrite the TOAST relation while the decoding worker waits +# for s2 to commit. +permutation + s2_begin + s1_wait_before_lock + s3_rewrite_toast(*) + s3_noop + s2_commit + s2_updates + s2_check + s2_wakeup_before_lock + s1_check -- 2.47.3