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

Hi Michael,

On Thu, Oct 22, 2026 at 11:22:08AM +0900, Michael Paquier wrote:
> The DELETE case is a much more common case. It regresses even more
> badly, and that does not look acceptable to me. That should be
> optimizable, of course, I don't really know yet how to do that.

I ran your scripts as attached upthread, on master 871dee0ccbe with
v2, 500k rows and 64 holders, 12 interleaved rounds per build. The
ratios match yours: VACUUM takes 1.98x HEAD with delete.sql (95% CI
1.96..2.02) and 1.53x with subxact_updates.sql (1.51..1.55), and
insert_only.sql does not move. Method and tables are in the attached
measurements.txt.

The two cases cost for different reasons. In delete.sql xmax is the
inserter's own xid, so the new TransactionIdIsInProgress(xmax) is a
second proc array scan, and pg_stat_slru shows no extra access. It
grows with the number of open transactions: 1.51x with no holder,
1.95x with 16, 2.02x with 128. In subxact_updates.sql xmax is one of
500k subxids, the subxid cache has overflowed, and v2 doubles the
pg_subtrans and clog accesses of each VACUUM (999872 to 1999744 for
pg_subtrans, 499936 to 999872 for the clog).

Zhijie's idea, doing the check only when xmin != xmax, removes the
delete.sql regression: 1.00x (0.99..1.01). In delete.sql xmax and
xmin are the same xid, so the check is skipped there. subxact_updates
stays at 1.53x, as he expected. The condition the other way around
loses the pg_class row again. On its own the idea keeps the
MultiXact race of his repro; with the HEAP_XMAX_IS_MULTI skip you
suggested, the repro passes. Both together are nocfbot-0001.

For subxact_updates, nocfbot-0002 on top of it. In that branch xmin
is in progress, so xmax is the inserter's xid or one of its
subtransactions, and RecordTransactionAbort() writes a subtransaction
abort to the clog before the xids leave the proc array. So
TransactionIdDidAbort(xmax) gives the same answer there as
!TransactionIdIsInProgress() && !TransactionIdDidCommit(), with one
clog lookup and no proc array or pg_subtrans access. With both
diffs, subxact_updates is at 1.01x (1.00..1.03) with 64 holders and
1.00x with 128. What 0002 costs is that clog lookup per tuple:
1.14x with no holder at all (50 ms to 57 ms).

With 0001 alone and with 0001 + 0002, these pass: the v2-0002 spec,
your pg_statistic sequence, VACUUM FULL pg_class with the CREATE
TABLE transaction open, logical decoding across it, Zhijie's
injection point repro, make check, isolation, and recovery 035.
Since 0002 changes when the hint is set, I added five cases where
the pg_class row must survive VACUUM FULL pg_class exactly once
(released, nested and prepared subtransactions). A build that sets
the hint unconditionally passes the original SQL cases and fails
three of the new ones, with two pg_class rows for the table.

The diffs apply on top of v2-0001 and v2-0002. Shihao, if they hold
up, feel free to fold them into a v3.

Regards,
Manu