Re: Handle infinite recursion in logical replication setup

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Handle infinite recursion in logical replication setup
Date: 2022-04-04 13:20:08
Message-ID: CAExHW5thMZ02d_y0aVA2aiCZnE+R2HCJREuOa=J8dnBmtKJiUg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I didn't find this in https://commitfest.postgresql.org/37/. Is this
somewhere in the commitfest?

On Fri, Apr 1, 2022 at 12:46 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, Mar 31, 2022 at 2:52 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Mar 31, 2022 at 9:14 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Mar 30, 2022 at 7:40 PM Ashutosh Bapat
> > > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > > > >
> > > > > The changes for the same are available int the v5 patch available at [1].
> > > > > [1] - https://www.postgresql.org/message-id/CALDaNm3wCf0YcvVo%2BgHMGpupk9K6WKJxCyLUvhPC2GkPKRZUWA%40mail.gmail.com
> > > > >
> > > >
> > > > cb->truncate_cb = pg_decode_truncate;
> > > > cb->commit_cb = pg_decode_commit_txn;
> > > > cb->filter_by_origin_cb = pg_decode_filter;
> > > > + cb->filter_remote_origin_cb = pg_decode_filter_remote_origin;
> > > >
> > > > Why do we need a new hook? Can we use filter_by_origin_cb?
> > > >
> > >
> > > I also think there is no need for a new hook in this case but I might
> > > also be missing something that Vignesh has in his mind.
> > >
> > > > Also it looks like
> > > > implementation of pg_decode_filter and pg_decode_filter_remote_origin is same,
> > > > unless my eyes are deceiving me.
> > > >
> > > > - <literal>binary</literal>, <literal>streaming</literal>, and
> > > > - <literal>disable_on_error</literal>.
> > > > + <literal>binary</literal>, <literal>streaming</literal>,
> > > > + <literal>disable_on_error</literal> and
> > > > + <literal>publish_local_only</literal>.
> > > >
> > > > "publish_local_only" as a "subscription" option looks odd. Should it be
> > > > "subscribe_local_only"?
> > > >
> > >
> > > I also think "subscribe_local_only" fits better here.
> > >
> >
> > About 0002 patch,
> > Commit message:
> > ------
> > If a subscription is created to Node1 from Node3 with publish_local_only
> > and copy_data as ON, then throw an error so that user can handle
> > creation of subscription with table having consistent data.
> > ------
> > Do you want to refer to Node2 instead of Node3 here as Node3 doesn't
> > make sense in the description?
>
> It should be Node3, I have added more details in the commit message
> mentioning about the scenario.
>
> > Also, you haven't explained anywhere in the patch why
> > 'publish_local_only' (or whatever we call it) won't work for initial
> > sync? IIUC, it is because we can identify origin changes only based on
> > WAL and initial sync directly copies data from the heap. Am, I missing
> > something here?
>
> I have added the explanation where we are throwing the error.
>
> The v6 patch at [1] has the changes for the same.
> [1] - https://www.postgresql.org/message-id/CALDaNm16eBx2BjLFjfFHSU4pb25HmgV--M692OPgH_91Dwn%3D2g%40mail.gmail.com
>
> Regards,
> Vignesh

--
Best Wishes,
Ashutosh Bapat

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2022-04-04 13:20:43 Re: [HACKERS] WIP aPatch: Pgbench Serialization and deadlock errors
Previous Message Robert Haas 2022-04-04 13:19:43 Re: PostgreSQL shutdown modes