Re: BUG #19839: Assert in ReorderBufferRestoreCleanup for aborted, spilled transaction

Hi,

On Sat, Nov 7, 2026 at 09:26 AM Atisss hh <125atishprak(at)gmail(dot)com>wrote:
>
>
> On Sat, 7 Nov 2026 at 06:54, PG Bug reporting form
> <noreply(at)postgresql(dot)org <mailto:noreply(at)postgresql(dot)org>> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference:      19839
> Logged by:          Atish Prakash
> Email address: 125atishprak(at)gmail(dot)com <mailto:125atishprak(at)gmail(dot)com>
> PostgreSQL version: 18.6
> Operating system:   Linux x86_64 (kernel 7.0.0, glibc 2.43)
> Description:
>
> On an assert-enabled build, logical decoding can fail this assertion and
> take
> the server through crash recovery:
>
>     TRAP: failed Assert("XLogRecPtrIsValid(txn->final_lsn)"), File:
> "reorderbuffer.c", Line: 5001
>
> Expected: decoding discards the aborted transaction and continues.
>
> The sequence is:
>
> 1. A transaction has a subtransaction with an XID but no decoded
> changes,
> for
>    example one that only ran SELECT ... FOR UPDATE.
> 2. The parent exceeds logical_decoding_work_mem while still in progress.
>    ReorderBufferSerializeTXN() recurses into the subtransaction, writes
> nothing,
>    and still sets RBTXN_IS_SERIALIZED on it. Its final_lsn stays
> invalid.
> 3. The parent aborts while the decoder is behind.
> 4. The parent is chosen for eviction before its abort record is decoded.
>    ReorderBufferCheckAndTruncateAbortedTXN() finds it aborted and
> truncates
> it,
>    and ReorderBufferRestoreCleanup() fails on the subtransaction's
> final_lsn.
>
> ReorderBufferCheckAndTruncateAbortedTXN() came in with 072ee847ad, so I
> expect
> 17 and earlier are not affected, but I have not tested them.

Thanks for the report and patch.

I think this issue affects can all supported branches (back to PG 14),
and does not require an aborted transaction to reproduce.

For example, if a top-level transaction has a subtransaction with an XID
but no decoded changes, and then spills to disk while queuing a partial
change (such as TOAST chunks), the empty subtransaction is marked
RBTXN_IS_SERIALIZED with final_lsn == InvalidXLogRecPtr. Once the main
table insert completes the partial change,
ReorderBufferProcessPartialChange() streams the transaction via
ReorderBufferStreamTXN(), and ReorderBufferTruncateTXN() ->
ReorderBufferRestoreCleanup() hits the same assertion on the empty
subtransaction.

See the attachment for a test to reproduce this issue. (Feel free to
merge if it's worth adding a test to cover this path)

>
> From reading the code, a non-assert build goes through an empty
> cleanup loop
> for
> this subtransaction, so I expect no user-visible effect there. I
> have not
> run a
> non-assert build.

I think in a rare case, this can also affect the stats collection for
spilled txns. For example, if an empty subtransaction is marked as
serialized, we skip incrementing spillTxns because no changes were
written; if real changes are later serialized in this subtransaction, we
still do not increment spillTxns because the subtransaction is already
marked as serialized and we avoid counting it "twice".

>
> Proposed fix, attached as 0001-fix-aborted-spilled-txn-assert.patch: set
> RBTXN_IS_SERIALIZED only if something was written.

And I have one minor comment for the patch:

+ if (spilled)
+ txn->txn_flags |= RBTXN_IS_SERIALIZED;

ReorderBufferSerializeTXN() already has an "if (spilled)" block right
above this, so we shall directly move the assignment into that block.

--
Best Regards,
Zhijie Hou