Horton

Re: Skipping logical replication transactions on subscriber side

Masahiko Sawada <sawada.mshk@gmail.com>

View thread View on postgr.es
From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, vignesh C <vignesh21@gmail.com>, Greg Nancarrow <gregn4422@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Alexey Lesovsky <lesovsky@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2022-01-21T03:14:05Z
Lists: pgsql-hackers 
On Wed, Jan 19, 2022 at 5:58 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Jan 19, 2022 at 12:46 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Jan 19, 2022 at 12:22 PM osumi.takamichi@fujitsu.com
> > <osumi.takamichi@fujitsu.com> wrote:
> > >
> > > (6) apply_handle_stream_abort
> > >
> > > @@ -1209,6 +1300,13 @@ apply_handle_stream_abort(StringInfo s)
> > >
> > >         logicalrep_read_stream_abort(s, &xid, &subxid);
> > >
> > > +       /*
> > > +        * We don't expect the user to set the XID of the transaction that is
> > > +        * rolled back but if the skip XID is set, clear it.
> > > +        */
> > > +       if (MySubscription->skipxid == xid || MySubscription->skipxid == subxid)
> > > +               clear_subscription_skip_xid(MySubscription->skipxid, InvalidXLogRecPtr, 0);
> > > +
> > >
> > > In my humble opinion, this still cares about subtransaction xid still.
> > > If we want to be consistent with top level transactions only,
> > > I felt checking MySubscription->skipxid == xid should be sufficient.
> >
> > I thought if we can clear subskipxid whose value has already been
> > processed on the subscriber with a reasonable cost it makes sense to
> > do that because it can reduce the possibility of the issue that XID is
> > wraparound while leaving the wrong in subskipxid.
> >
>
> I guess that could happen if the user sets some unrelated XID value.
> So, I think it should be okay to not clear this but we can add a
> comment in the code at that place that we don't clear subtransaction's
> XID as we don't support skipping individual subtransactions or
> something like that.

Agreed and added the comment in the latest patch[1].

Regards,

[1] https://www.postgresql.org/message-id/CAD21AoDOuNtvFUfU2wH2QgTJ6AyMXXh_vdA87qX0mUibdsrYTg%40mail.gmail.com

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/

Commits

  1. Test ALIGNOF_DOUBLE==4 compatibility under ALIGNOF_DOUBLE==8.

  2. Reorder subskiplsn in pg_subscription to avoid alignment issues.

  3. Add ALTER SUBSCRIPTION ... SKIP.

  4. Optionally disable subscriptions on error.

  5. Update docs of logical replication for commit 8d74fc96db.

  6. Respect permissions within logical replication.

  7. Fix regression test failure caused by commit 8d74fc96db.

  8. Add a view to show the stats of subscription workers.

  9. Add logical change details to logical replication worker errcontext.

  10. Rename LOGICAL_REP_MSG_STREAM_END to LOGICAL_REP_MSG_STREAM_STOP.

  11. Fix typo in protocol.sgml.

  12. Remove unused argument in apply_handle_commit_internal().

  13. Fix replication of in-progress transactions in tablesync worker.

  14. Reorder pg_sequence columns to avoid alignment issue