Re: Handle infinite recursion in logical replication setup

On Tue, Mar 15, 2022 at 7:09 AM kuroda.hayato@fujitsu.com
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Vignesh,
>
> Thank you for updating your patch!
>
> > Let's consider an existing Multi master logical replication setup
> > between Node1 and Node2 that is created using the following steps:
> > a) Node1 - Publication publishing employee table - pub1
> > b) Node2 - Subscription subscribing from publication pub1 with
> > publish_local_only - sub1_pub1_node1
> > c) Node2 - Publication publishing employee table - pub2
> > d) Node1 - Subscription subscribing from publication pub2 with
> > publish_local_only - sub2_pub2_node2
> >
> > To create a subscription in node3, we will be using the following steps:
> > a) Node2 - Publication publishing employee table. - pub3
> > b) Node3 - Subscription subscribing from publication in Node2 with
> > publish_local_only - sub3_pub3_node2
> >
> > When we create a subscription in Node3, Node3 will connect to
> > Node2(this will not be done in Node3) and check if the employee table
> > is present in pg_subscription_rel, in our case Node2 will have
> > employee table present in pg_subscription_rel (sub1_pub1_node1
> > subscribing to employee table from pub1 in Node1). As employee table
> > is being subscribed in node2 from node1, we will throw an error like
> > below:
> > postgres=# create subscription sub2 CONNECTION 'dbname =postgres port
> > = 9999' publication pub2 with (publish_local_only=on);
> > ERROR:  CREATE/ALTER SUBSCRIPTION with publish_local_only and
> > copy_data as true is not allowed when the publisher might have
> > replicated data, table:public.t1 might have replicated data in the
> > publisher
> > HINT:  Use CREATE/ALTER SUBSCRIPTION with copy_data = off or force
>
> Thanks for kind explanation.
> I read above and your doc in 0002, and I put some comments.
>
> 1. alter_subscription.sgml
>
> ```
> -        <term><literal>copy_data</literal> (<type>boolean</type>)</term>
> +        <term><literal>copy_data</literal> (<type>boolean</type> | <literal>force</literal>)</term>
> ```
>
> I thought that it should be written as enum. For example, huge_pages GUC parameter
> can accept {on, off, try}, and it has been written as enum.

Modified

> 2. create_subscription.sgml
>
> ```
> -        <term><literal>copy_data</literal> (<type>boolean</type>)</term>
> +        <term><literal>copy_data</literal> (<type>boolean</type> | <literal>force</literal>)</term>
> ```
>
> Same as above.

Modified

> 3. create_subscription.sgml
>
> ```
> +
> +         <para>
> +          If the publication tables were also subscribing data in the publisher
> +          from other publishers, it will affect the
> +          <command>CREATE SUBSCRIPTION</command> based on the value specified
> +          for <literal>publish_local_only</literal> option. Refer to the
> +          <xref linkend="sql-createsubscription-notes" /> for details.
> +         </para>
> ```
>
> I seeked docs, but the words " publication tables " have not seen.
> How about "tables in the publication"?

Modified

> 4. create_subscription.sgml - about your example
>
> In the first section, we should describe about 2-nodes case more detail
> like Amit mentioned in [1]. I thought that Option-3 can be resolved by defining
> subscriptions in both nodes with publish_local_only = true and copy_data = force.

I thought existing information is enough because we have mentioned
that node1 and node2 have bidirectional replication setup done and
both the table data will be replicated and synchronized as and when
the DML operations are happening. In option-3 we need to create a
subscription with copy_data as force to one node and copy_data as
false to another node because both nodes will be having the same data,
copying the data just from one of the nodes should be enough.

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

Regards,
Vignesh