Hi, On 2018-10-22 12:03:26 +0900, Michael Paquier wrote: > (moving to -hackers) > > On Sun, Oct 14, 2018 at 10:42:40AM -0700, Andres Freund wrote: > > I'm unhappy this approach was taken over objections. Without a real > > warning. > > Oops, that was not clear to me. Sorry about that! I did not see you > objecting again after the last arguments I raised. The end of PREPARE > TRANSACTION is designed like that since it has been introduced, so > changing the way the dummy GXACT is inserted before the main one is > cleared from its XID does not sound wise to me. The current design is > also present for a couple of reasons, please see this thread: > https://www.postgresql.org/message-id/13905.1119109...@sss.pgh.pa.us > This has resulted in e26b0abd.
None of them explains why having "corrupt" WAL that's later fixed up is a good idea. > Among the things I thought are: > - Clearing the XID at the same time the dummy entry is added, which > actually means to hold on ProcArrayLock longer while doing more at the > end of prepare. I actually don't think you can do that cleanly without > endangering the transaction visibility for other backends, and syncrep > may cause the window to get wider. > - Changing GetRunningTransactionData so as duplicates are removed at > this stage. However this also requires to hold ProcArrayLock for > longer. That's *MUCH* better than what we have right now. GetRunningTransactionData() isn't called all that often, for once, and for another the WAL then is correct. > > Even leaving the crummyness aside, did you check other users of > > XLOG_RUNNING_XACTS, e.g. logical decoding? > > I actually spent some time checking that, so it is not innocent. "innocent"? > SnapBuildWaitSnapshot() waits for transactions to commit or abort based > on the XIDs in the record. And that's the only place where those XIDs > are used so it won't matter to wait twice for the same transaction to > finish. The error callback would be used only once. Right. We used to use it more (and it'd probably caused problems), but since 955a684e0401954a58e956535107bc4b7136d952 it should be ok... Greetings, Andres Freund
