On Mon, Nov 23, 2020 at 3:41 PM Ajin Cherian <itsajin@gmail.com> wrote:
>
> On Sun, Nov 22, 2020 at 12:31 AM Amit Kapila <amit.kapila16@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.