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

Hi,

On Sun, Nov 08, 2026 at 11:11:09PM +0800, Zhijie Hou wrote:
> Thanks for the report and patch.

+1.

> 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)

> 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".

Agreed.

That's why I think the test is worth adding (as the bug could also produce real
user visible impact as it can cause spill_txns to be undercounted on non assert
enabled builds). The test only detects the regression on assert enabled builds,
but I think that is enough. A test for the spill_txns case would be more
complicated and is probably not worth it.

>
> + 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.

+1.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com