On Tue, Jan 5, 2021 at 12:32 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Jan 05, 2021 at 09:35:21AM +0530, Amit Kapila wrote:
> > As noted in [1], without this the subscriber might again ask for
> > rollback prepared lsn after restart.
> >
> > Attached patch addresses this problem.
>
> Is it possible to add some tests in test_decoding?
>
There are already tests [1] in one of the upcoming patches for logical
decoding of 2PC which covers this code using which I have found this
problem. So, I thought those would be sufficient. I have not checked
the feasibility of using test_decoding because I thought adding more
using test_decoding will unnecessarily duplicate the tests.
> /* dump transaction origin information only for abort prepared */
> if ((replorigin_session_origin != InvalidRepOriginId) &&
> - TransactionIdIsValid(twophase_xid) &&
> - XLogLogicalInfoActive())
> + TransactionIdIsValid(twophase_xid))
> It seems to me that you may want to document as a comment the reason
> why this gets sent even if (wal_level < logical).
>
How about something like "Dump transaction origin information only for
abort prepared. We need this during recovery to update the replication
origin progress."?
[1] - See the tests with comments "check that 2PC gets replicated to
subscriber then ROLLBACK PREPARED" and "Check that ROLLBACK PREPARED
is decoded properly on crash restart (publisher and subscriber crash)"
in v33-0007-Support-2PC-txn-subscriber-tests at
https://www.postgresql.org/message-id/CAA4eK1L3p4z%2B9wtK77MbdpkagR4GS2Y3r1Je7ZEvLQVF9GArfg%40mail.gmail.com
--
With Regards,
Amit Kapila.
