Hi hackers, While testing prepared transactions with subtransactions, I saw a prepared transaction whose changes were all sent downstream even though it had been rolled back while it was being decoded. I found that the concurrent-abort check in ReorderBufferProcessTXN() is skipped for changes made in a subtransaction of a prepared transaction.
When decoding a prepared transaction, ReorderBufferProcessTXN() is meant to
set CheckXidAlive for each change, so a catalog scan notices if the
transaction aborts concurrently. The check does SetupCheckXidLive here:
if (streaming || rbtxn_is_prepared(change->txn))
{
curtxn = change->txn;
SetupCheckXidLive(curtxn->xid);
}
RBTXN_IS_PREPARED is set only on the top-level transaction, in
ReorderBufferRememberPrepareInfo(). For a change made in a subtransaction,
change->txn is the subtransaction's own ReorderBufferTXN, so the test is
false and CheckXidAlive is not set for it. It stays invalid, unless an
earlier top-level change set it. Because it is invalid,
HandleConcurrentAbort() does nothing, so decoding carries on and sends the
changes. The comment above SetupCheckXidLive() says the check covers the
"(sub)transaction", so I believe this is unintended.
The attached patch tests the flag on the top-level transaction:
if (streaming || rbtxn_is_prepared(rbtxn_get_toptxn(change->txn)))
Once the transaction is prepared, its subtransactions commit or abort
together with it. The error handler already tests rbtxn_is_prepared(txn) on
the top-level transaction, so it needs no change.
The patch adds an injection point, and a test using test_decoding that
fails without the fix and passes with it.
I searched the archives but did not see any earlier report. If there is
one, kindly point me to it.
Regards,
Vaijayanti
v1-0001-Fix-concurrent-abort-detection-for-subtransaction.patch
Description: Binary data
