Re: Skipping logical replication transactions on subscriber side

On Mon, Jul 19, 2021 at 8:38 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On July 19, 2021 2:40 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > I've attached the updated version patch that incorporated all comments
> > I got so far except for the clearing error details part I mentioned
> > above. After getting a consensus on those parts, I'll incorporate the
> > idea into the patches.
>
> Hi Sawada-san,
>
> I am interested in this feature.
> After having a look at the patch, I have a few questions about it.

Thank you for having a look at the patches!

>
> 1) In 0002 patch, it introduces a new view called pg_stat_subscription_errors.
> Since it won't be cleaned automatically after we resolve the conflict, do we
> need a reset function to clean the statistics in it ? Maybe something
> similar to pg_stat_reset_replication_slot which clean the
> pg_stat_replication_slots.

Agreed. As Amit also mentioned, providing a reset function to clean
the statistics seems a good idea. If the message clearing the stats
that is sent after skipping the transaction gets lost, the user is
able to reset those stats manually.

>
> 2) For 0003 patch, When I am faced with a conflict, I set skip_xid = xxx, and
> then I resolve the conflict. If I reset skip_xid after resolving the
> conflict, will the change(which cause the conflict before) be applied again ?

The apply worker checks skip_xid when it reads the subscription.
Therefore, if you reset skip_xid before the apply worker restarts and
skips the transaction, the change is applied. But if you reset
skip_xid after the apply worker skips transaction, the change is
already skipped and your resetting skip_xid has no effect.

>
> 3) For 0003 patch, if user set skip_xid to a wrong xid which have not been
> assigned, and then will the change be skipped when the xid is assigned in
> the future even if it doesn't cause any conflicts ?

Yes. Currently, setting a correct xid is the user's responsibility. I
think it would be better to disable it or emit WARNING/ERROR when the
user mistakenly set the wrong xid if we find out a convenient way to
detect that.

>
> Besides, It might be better to add some description of patch in each patch's
> commit message which will make it easier for new reviewers to follow.

I'll add commit messages in the next version patch.

Regards,

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