From 092dc34678cd151de4262322bda1b2d31b1cec11 Mon Sep 17 00:00:00 2001 From: Atish Prakash <125atishprak@gmail.com> Date: Sun, 8 Nov 2026 21:06:11 +0530 Subject: [PATCH v2] Don't mark a transaction serialized when nothing was spilled ReorderBufferSerializeTXN() set RBTXN_IS_SERIALIZED on every transaction it visited, including subtransactions with no changes to write. Such a transaction keeps an invalid final_lsn, because final_lsn is only advanced as changes are written, so ReorderBufferRestoreCleanup() can later fail its assertion on it. This happens when ReorderBufferCheckAndTruncateAbortedTXN() truncates an aborted transaction, and when a transaction is truncated after being streamed once a partial change completes. It also made spill_txns undercount: when such a subtransaction later spilled real changes, it was not counted, as it was already marked serialized. Set the flag only when changes were written, after the statistics update that checks it. Test case by Zhijie Hou. Bug: #19839 Reported-by: Atish Prakash <125atishprak@gmail.com> Author: Atish Prakash <125atishprak@gmail.com> Reviewed-by: Zhijie Hou Backpatch-through: 14 --- contrib/test_decoding/expected/stream.out | 24 +++++++++++++++++++ contrib/test_decoding/sql/stream.sql | 15 ++++++++++++ .../replication/logical/reorderbuffer.c | 11 ++++++++- 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/contrib/test_decoding/expected/stream.out b/contrib/test_decoding/expected/stream.out index 0ec5c93361..9c164847b0 100644 --- a/contrib/test_decoding/expected/stream.out +++ b/contrib/test_decoding/expected/stream.out @@ -157,6 +157,30 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc COMMIT (3 rows) +-- bug #19839 +-- +-- A subtransaction without decoded changes must not be marked as serialized +-- when its parent transaction spills to disk during a partial TOAST change +-- (stream_test.data has EXTERNAL storage set above). Otherwise, truncating +-- the transaction after streaming fails on the subtransaction's invalid +-- final_lsn. +BEGIN; +SAVEPOINT s; +SELECT 1 FROM stream_test LIMIT 1 FOR UPDATE; + ?column? +---------- + 1 +(1 row) + +RELEASE SAVEPOINT s; +INSERT INTO stream_test VALUES (repeat('a', 100000)); +COMMIT; +SELECT count(*) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1', 'stream-changes', '1'); + count +------- + 4 +(1 row) + DROP TABLE stream_test; SELECT pg_drop_replication_slot('regression_slot'); pg_drop_replication_slot diff --git a/contrib/test_decoding/sql/stream.sql b/contrib/test_decoding/sql/stream.sql index 5e45a8e9b6..f315933556 100644 --- a/contrib/test_decoding/sql/stream.sql +++ b/contrib/test_decoding/sql/stream.sql @@ -82,5 +82,20 @@ ROLLBACK; INSERT INTO stream_test VALUES ('after-abort'); SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); +-- bug #19839 +-- +-- A subtransaction without decoded changes must not be marked as serialized +-- when its parent transaction spills to disk during a partial TOAST change +-- (stream_test.data has EXTERNAL storage set above). Otherwise, truncating +-- the transaction after streaming fails on the subtransaction's invalid +-- final_lsn. +BEGIN; +SAVEPOINT s; +SELECT 1 FROM stream_test LIMIT 1 FOR UPDATE; +RELEASE SAVEPOINT s; +INSERT INTO stream_test VALUES (repeat('a', 100000)); +COMMIT; +SELECT count(*) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1', 'stream-changes', '1'); + DROP TABLE stream_test; SELECT pg_drop_replication_slot('regression_slot'); diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 4684398531..acbd2c1344 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -4211,12 +4211,21 @@ ReorderBufferSerializeTXN(ReorderBuffer *rb, ReorderBufferTXN *txn) /* update the decoding stats */ UpdateDecodingStats((LogicalDecodingContext *) rb->private_data); + + /* + * Mark the transaction as serialized only if something was written, + * and only after the check above. final_lsn is advanced for each + * change written, so a transaction that wrote nothing, such as a + * subtransaction without decoded changes, would be marked with an + * invalid final_lsn, and ReorderBufferRestoreCleanup() would later + * fail on it. + */ + txn->txn_flags |= RBTXN_IS_SERIALIZED; } Assert(spilled == txn->nentries_mem); Assert(dlist_is_empty(&txn->changes)); txn->nentries_mem = 0; - txn->txn_flags |= RBTXN_IS_SERIALIZED; if (fd != -1) CloseTransientFile(fd); -- 2.53.0