Re: BUG #19760: CLUSTER loses a row the transaction inserted and then updated in a rolled-back subtransaction

Hi Zhijie,

On Sun, Oct 25, 2026 at 4:31 PM Zhijie Hou <houzhijie22(at)gmail(dot)com>
wrote:
> This doesn't look safe to me. Even if TransactionIdDidAbort() returns true, the
> xmax may not have been removed from PGPROC yet. So this change would create the
> risky case where the old tuple has been marked as XMAX_INVAL while the new
> tuple's xmin is still considered in progress.

You're right. On master dfd7ede529f with v2 and nocfbot-0001, your
test fails in 3 of 3 runs once nocfbot-0002 is added ("failed to find
parent tuple for heap-only tuple at (0,3)"), and passes in 4 of 4
without it.

I took the order in RecordTransactionAbort() (clog first, proc array
later) as the reason the clog alone was enough, but that order is
what opens the window: until ProcArrayEndTransaction() runs, the new
version of the tuple, whose xmin is that xmax, is still in progress,
which is the race the note at the top of heapam_visibility.c is
about.

So the clog can only be a filter there. The attached diff replaces
nocfbot-0002: it keeps TransactionIdDidAbort(xmax) first and then
confirms with !TransactionIdIsInProgress(xmax) before setting the
hint. It passes your test in 4 of 4 runs, shihao's spec, cases 1-3,
5 and 7-11 from measurements.txt, and make check, the isolation suite,
injection_points and test_decoding.

It keeps what made nocfbot-0002 cheap in subxact_updates.sql. There
xmax is a released subtransaction that is still running, so it is
not aborted in the clog and the proc array is never asked. One
VACUUM makes the same SLRU accesses as with the old nocfbot-0002:
999937 clog and 999872 subtrans, against 999873 and 1999744 with
nocfbot-0001 alone. When xmax did abort, it pays both lookups until
the hint is set. I have not re-timed it.

nocfbot-0001 alone (your xmin != xmax idea plus the MULTI skip) is
unchanged and passes your test in 4 of 4 runs.

Thanks for the repro.

Regards,
Manu