Re: Single transaction in the tablesync worker?

On Fri, Dec 4, 2020 at 7:53 AM Craig Ringer
<craig(dot)ringer(at)enterprisedb(dot)com> wrote:
>
> On Thu, 3 Dec 2020 at 17:25, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
>
> > The reason why I am looking into this area is to support the logical
> > decoding of prepared transactions. See the problem [1] reported by
> > Peter Smith. Basically, when we stream prepared transactions in the
> > tablesync worker, it will simply commit the same due to the
> > requirement of maintaining a single transaction for the entire
> > duration of copy and streaming of transactions. Now, we can fix that
> > problem by disabling the decoding of prepared xacts in tablesync
> > worker.
>
> Tablesync should indeed only receive a txn when the commit arrives, it
> should not attempt to handle uncommitted prepared xacts.
>

Why? If we go with the approach of the commit as we go for individual
transactions in the tablesync worker then this shouldn't be a problem.

> > But that will arise to a different kind of problems like the
> > prepare will not be sent by the publisher but a later commit might
> > move lsn to a later step which will allow it to catch up till the
> > apply worker. So, now the prepared transaction will be skipped by both
> > tablesync and apply worker.
>
> I'm not sure I understand. If what you describe is possible then
> there's already a bug in prepared xact handling. Prepared xact commit
> progress should be tracked by commit lsn, not by prepare lsn.
>

Oh no, I am talking about commit of some other transaction.

> Can you set out the ordering of events in more detail?
>

Sure. It will be something like where apply worker is ahead of sync worker:

Assume t1 has some data which tablesync worker has to first copy.

tx1
Begin;
Insert into t1....
Prepare Transaction 'foo'

tx2
Begin;
Insert into t1....
Commit

apply worker
• tx1: replays - does not apply anything because
should_apply_changes_for_rel thinks relation is not ready
• tx2: replays - does not apply anything because
should_apply_changes_for_rel thinks relation is not ready

tablesync worder
• tx1: handles: BEGIN - INSERT - PREPARE 'xyz'; (but tablesync gets
nothing because say we disable 2-PC for it)
• tx2: handles: BEGIN - INSERT - COMMIT;
• tablelsync exits

Now the situation is that the apply worker has skipped the prepared
xact data and tablesync worker has not received it, so not applied it.
Next, when we get Commit Prepared for tx1, it will silently commit the
prepared transaction without any data being updated. The commit
prepared won't error out in subscriber because the prepare would have
been successful even though the data is skipped via
should_apply_changes_for_rel.

> > I think apart from unblocking the development of 'logical decoding of
> > prepared xacts', it will make the code consistent between apply and
> > tablesync worker and reduce the chances of future bugs in this area.
> > Basically, it will reduce the checks related to am_tablesync_worker()
> > at various places in the code.
>
> I think we made similar changes in pglogical to switch to applying
> sync work in individual txns.
>

oh, cool. Did you make some additional changes as you have mentioned
in the earlier part of the email?

--
With Regards,
Amit Kapila.