On Mon, Nov 23, 2020 at 3:41 PM Ajin Cherian <itsa...@gmail.com> wrote: > > On Sun, Nov 22, 2020 at 12:31 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > I am planning to continue review of these patches but I thought it is > > better to check about the above changes before proceeding further. Let > > me know what you think? > > > > I've had a look at the changes and done a few tests, and have no > comments. >
Okay, thanks. Additionally, I have analyzed whether we need to call SnapbuildCommittedTxn in DecodePrepare as was raised earlier for this patch [1]. As mentioned in that thread SnapbuildCommittedTxn primarily does three things (a) Decide whether we are interested in tracking the current txn effects and if we are, mark it as committed. (b) Build and distribute snapshot to all RBTXNs, if it is important. (c) Set base snap of our xact if it did DDL, to execute invalidations during replay. For the first two, as the xact is still not visible to others so we don't need to make it behave like a committed txn. To make the (DDL) changes visible to the current txn, the message REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID copies the snapshot which fills the subxip array. This will be sufficient to make the changes visible to the current txn. For the third, I have checked the code that whenever we have any change message the base snapshot gets set via SnapBuildProcessChange. It is possible that I have missed something but I don't want to call SnapbuildCommittedTxn in DecodePrepare unless we have a clear reason for the same so leaving it for now. Can you or someone see any reason for the same? > However, I did see that the test 002_twophase_streaming.pl > failed once. I've run it at least 30 times after that but haven't seen > it fail again. > This test is based on waiting to see some message in the log. It is possible it failed due to timeout which can only happen rarely. You can check some failure logs in test_decoding folder (probably in tmp_check folder). Even if we get some server or test log, it can help us to diagnose the problem. [1] - https://www.postgresql.org/message-id/87zhxrwgvh.fsf%40ars-thinkpad -- With Regards, Amit Kapila.