Hi! While playing around with two-phase transactions, I've noticed something strange. The prepare_start_lsn and prepare_end_lsn fields in gxact are modified within CheckPointTwoPhase() without the use of proper locking, which means they could become inconsistent.
The original commit by Tom (e26b0abda391, from 2005) only applied a SHARED lock. Later, Simon Riggs added commit 728bd991c3c4, "Speedup 2PC recovery," which moved the invalidating of these fields from the MarkAsPreparing() to CheckPointTwoPhase(), but no locking was implemented in this new location. I suspected that this issue would be easy to reproduce using injection points, and that turned out to be the case. There are three problematic places: the calls to FinishPreparedTransaction(), EndPrepare(), and LookupGXact(). In one instance, running the tests triggers an assertion failure; in others, the logic governing two-phase transactions is compromised. I totally understand that this is a *very* rare scenario in real-world use, but it is still something worth being aware of. As for a fix, I’ve implemented some ad-hoc solutions, PFA patch set. Actually, I don't really understand why the invalidation of the prepare_start_lsn and prepare_end_lsn fields is required within CheckPointTwoPhase(). Is it possible to get rid of them entirely without breaking something? Thoughts? -- Best regards, Maxim Orlov.
v1-0002-Add-2pc-race-test-between-CheckPointTwoPhase-and-.patch
Description: Binary data
v1-0003-Add-2pc-race-test-between-CheckPointTwoPhase-and-.patch
Description: Binary data
v1-0001-Add-2pc-race-test-between-CheckPointTwoPhase-and-.patch
Description: Binary data
