Re: Handle infinite recursion in logical replication setup

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(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-07-09 14:40:56
Message-ID: CALDaNm0B3qh12hJya1sAmsmD916jBqin4-96ihBNX7f1qOppRw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 8, 2022 at 4:39 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Jul 5, 2022 at 9:33 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > Since the existing test is already handling the verification of this
> > scenario, I felt no need to add the test. Updated v29 patch removes
> > the 0001 patch which had the test case.
> >
>
> I am not able to apply 0001.
> patching file src/bin/psql/tab-complete.c
> Hunk #1 FAILED at 1873.
> Hunk #2 FAILED at 3152.

I have rebased the patch on top of HEAD.

> Few comments on 0002
> =====================
> 1.
> + <xref linkend="sql-createsubscription-notes" /> for interaction
>
> Is there a need for space before / in above? If not, please remove it
> with similar extra space from other similar usages.

Modified

> 2.
> + <para>
> + There is some interaction between the <literal>origin</literal>
> + parameter and the <literal>copy_data</literal> parameter. Refer to
> + the <command>CREATE SUBSCRIPTION</command>
> + <xref linkend="sql-createsubscription-notes" /> for interaction
> + details and usage of <literal>force</literal> for
> + <literal>copy_data</literal> parameter.
> </para>
>
> I think this is bit long. We can try to reduce this by something like:
> Refer <xref linkend="sql-createsubscription-notes"/> for the usage of
> <literal>force</literal> option and its interaction with the
> <literal>origin</literal> parameter.
>
> Also, adopt the same other places if you agree with the above change.

Modified with slight rewording.

> 4.
> @@ -601,16 +549,28 @@ GetSubscriptionNotReadyRelations(Oid subid)
> SysScanDesc scan;
>
> rel = table_open(SubscriptionRelRelationId, AccessShareLock);
> -
> ScanKeyInit(&skey[nkeys++],
> Anum_pg_subscription_rel_srsubid,
>
> Spurious line removal.

I have removed this

> 5.
> +static void
> +check_pub_table_subscribed(WalReceiverConn *wrconn, List *publications,
> + CopyData copydata, char *origin, Oid subid)
> {
> ...
> ...
> + /*
> + * Get the ready relations for the subscription. The subid will be valid
> + * only for ALTER SUBSCRIPTION ... REFRESH because there will be no
> + * relations in ready state while the subscription is created.
> + */
> + if (OidIsValid(subid))
> + subreadyrels = GetSubscriptionRelations(subid, READY_STATE);
>
> Why do we want to consider only READY_STATE relations here? If you see
> its caller AlterSubscription_refresh(), we don't consider copying the
> relation if it exists on subscribers in any state. If my observation
> is correct then you probably don't need to introduce SubRelStateType.

I have changed it similar to the caller function and removed SubRelStateType

Thanks for the comments, the v30 patch attached has the changes for the same.

Regards,
Vignesh

Attachment Content-Type Size
v30-0003-Document-bidirectional-logical-replication-steps.patch text/x-patch 13.5 KB
v30-0002-Check-and-throw-an-error-if-publication-tables-w.patch text/x-patch 41.1 KB
v30-0001-Skip-replication-of-non-local-data.patch text/x-patch 57.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-07-09 15:03:54 Re: automatically generating node support functions
Previous Message Peter Eisentraut 2022-07-09 14:37:22 Re: automatically generating node support functions