Re: Skipping logical replication transactions on subscriber side

On Thu, Mar 17, 2022 at 8:13 AM shiy(dot)fnst(at)fujitsu(dot)com
<shiy(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Wed, Mar 16, 2022 4:23 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > I've attached an updated version patch.
> >
>
> Thanks for updating the patch. Here are some comments for the v15 patch.
>
> 1. src/backend/replication/logical/worker.c
>
> + * to skip applying the changes when starting to apply changes. The subskiplsn is
> + * cleared after successfully skipping the transaction or applying non-empty
> + * transaction. The latter prevents the mistakenly specified subskiplsn from
>
> Should "applying non-empty transaction" be modified to "finishing a
> transaction"? To be consistent with the description in the
> alter_subscription.sgml.
>

The current wording in the patch seems okay to me as it is good to
emphasize on non-empty transactions.

> 2. src/test/subscription/t/029_on_error.pl
>
> +# Test of logical replication subscription self-disabling feature.
>
> Should we add something about "skip logical replication transactions" in this
> comment?
>

How about: "Tests for disable_on_error and SKIP transaction features."?

I am making some other minor edits in the patch and will take care of
whatever we decide for these comments.

--
With Regards,
Amit Kapila.