Hi,
As reported by Robert[1], it is worth adding a test for the race condition in
the RecordTransactionCommitPrepared() function to reduce the risk of future code
changes:
/*
* Note it is important to set committs value after marking ourselves as
* in the commit critical section (DELAY_CHKPT_IN_COMMIT). This is because
* we want to ensure all transactions that have acquired commit timestamp
* are finished before we allow the logical replication client to advance
* its xid which is used to hold back dead rows for conflict detection.
* See comments atop worker.c.
*/
committs = GetCurrentTimestamp();
While writing the test, I noticed a bug that conflict-relevant data could be
prematurely removed before applying prepared transactions on the publisher that
are in the commit phase. This occurred because GetOldestActiveTransactionId()
was called on the publisher, which failed to account for the backend executing
COMMIT PREPARED, as this backend does not have an xid stored in PGPROC.
Since this issue overlaps with the race condition related to timestamp
acquisition, I've prepared a bug fix along with a test for the race condition.
The 0001 patch fixes this issue by introducing a new function that iterates over
global transactions and identifies prepared transactions during the commit
phase. 0002 added injection points and tap-test to test the bug and timestamp
acquisition.
[1] https://www.postgresql.org/message-id/CA%2BTgmoaQtB%3DcnMJwCA33bDrGt7x5ysoW770uJ2Z56AU_NVNdbw%40mail.gmail.com
Best Regards,
Hou zj
