Re: Skipping logical replication transactions on subscriber side

On Wed, Aug 11, 2021 at 5:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Aug 11, 2021 at 11:19 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Tue, Aug 10, 2021 at 7:18 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > ==============================================================
> > >
> > > 1. While applying DML operations, we are setting up the error context
> > > multiple times due to which the context information is not
> > > appropriate. The first is set in apply_dispatch and then during
> > > processing, we set another error callback slot_store_error_callback in
> > > slot_store_data and slot_modify_data. When I forced one of the errors
> > > in slot_store_data(), it displays the below information in CONTEXT
> > > which doesn't make much sense.
> > >
> > > 2021-08-10 15:16:39.887 IST [6784] ERROR: incorrect binary data
> > > format in logical replication column 1
> > > 2021-08-10 15:16:39.887 IST [6784] CONTEXT: processing remote data
> > > for replication target relation "public.test1" column "id"
> > > during apply of "INSERT" for relation "public.test1" in
> > > transaction with xid 740 committs 2021-08-10 14:44:38.058174+05:30
> >
> > Yes, but we cannot change the error context message depending on other
> > error context messages. So it seems hard to construct a complete
> > sentence in the context message that is okay in terms of English
> > grammar. Is the following message better?
> >
> > CONTEXT: processing remote data for replication target relation
> > "public.test1" column “id"
> > applying "INSERT" for relation "public.test1” in transaction
> > with xid 740 committs 2021-08-10 14:44:38.058174+05:30
> >
>
> I don't like the proposed text. How about if we combine both and have
> something like: "processing remote data during "UPDATE" for
> replication target relation "public.test1" column "id" in transaction
> id 740 with commit timestamp 2021-08-10 14:44:38.058174+05:30"? For
> this, I think we need to remove slot_store_error_callback and
> add/change the ApplyErrCallbackArg to include the additional required
> information in that callback.

Oh, I've never thought about that. That's a good idea.

I've attached the updated patches. FYI I've included the patch
(v8-0005) that fixes the assertion failure during shared fileset
cleanup to make cfbot tests happy.

Regards,

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