Re: Handle infinite recursion in logical replication setup

On Tue, Jul 26, 2022 at 7:16 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Here are some review comments for the patch v38-0002:
>
> ======
>
> <General> - terminology
>
> There seemed to be an inconsistent alternation of the terms
> "primaries" and "nodes"... For example "Setting replication between
> two primaries" versus "Adding a new node..." (instead of "Adding a new
> primary..."?). I have included suggested minor rewording changes in
> the review comments below, but please check in case I miss something.
> Because I suggested changes to some titles maybe you will also want to
> change the section ids too.
>
> ~~~
>
> 1. Commit message
>
> The documentation was recently modified to remove the term
> "bidirectional replication" and replace it all with "replication
> between primaries", so this commit message (and also the patch name
> itself) should be similarly modified.

Modified

> ~~~
>
> 2.
> +   <para>
> +    Replication between primaries is useful for creating a multi-master
> +    database environment for replicating write operations performed by any of
> +    the member nodes. The steps to create replication between primaries in
> +    various scenarios are given below. Note: User is responsible for designing
> +    their schemas in a way to minimize the risk of conflicts. See
> +    <xref linkend="logical-replication-conflicts"/> for the details of logical
> +    replication conflicts. The logical replication restrictions applies to
> +    the replication between primaries also. See
> +    <xref linkend="logical-replication-restrictions"/> for the details of
> +    logical replication restrictions.
> +   </para>
>
> 2a.
> "User" -> "The user"

Modified

> 2b.
> "The logical replication restrictions applies to..." --> "The logical
> replication restrictions apply to..."

Modified

> 2c.
> These are important notes. Instead of just being part of the text
> blurb, perhaps these should be rendered as SGML <note> (or put them
> both in a single <note> if you want)

Modified

> ~~~
>
> 3. Setting replication between two primaries
>
> +   <title>Setting replication between two primaries</title>
> +   <para>
> +    The following steps demonstrate how to setup replication between two
> +    primaries when there is no table data present on both nodes
> +    <literal>node1</literal> and <literal>node2</literal>:
> +   </para>
>
> SUGGESTED
> The following steps demonstrate how to set up replication between two
> primaries (node1 and node2) when there is no table data present on
> both nodes:.

Modified

> ~~~
>
> 4.
> +   <para>
> +    Now the replication setup between two primaries <literal>node1</literal>
> +    and <literal>node2</literal> is complete. Any incremental changes from
> +    <literal>node1</literal> will be replicated to <literal>node2</literal>,
> +    and any incremental changes from <literal>node2</literal> will be
> +    replicated to <literal>node1</literal>.
> +   </para>
>
> "between two primaries" -> "between primaries"

Modified

> ~~~
>
> 5. Adding a new node when there is no table data on any of the nodes
>
> SUGGESTION (title)
> Adding a new primary when there is no table data on any of the primaries

Modified

> ~~~
>
> 6.
> +   <para>
> +    The following steps demonstrate adding a new node <literal>node3</literal>
> +    to the existing <literal>node1</literal> and <literal>node2</literal> when
> +    there is no <literal>t1</literal> data on any of the nodes. This requires
>
> SUGGESTION
> The following steps demonstrate adding a new primary (node3) to the
> existing primaries (node1 and node2) when there is no t1 data on any
> of the nodes.

Modified

> ~~~
>
> 7. Adding a new node when table data is present on the existing nodes
>
> SUGGESTION (title)
> Adding a new primary when table data is present on the existing primaries

Modified

> ~~~
>
> 8.
> +    <para>
> +     The following steps demonstrate adding a new node <literal>node3</literal>
> +     which has no <literal>t1</literal> data to the existing
> +     <literal>node1</literal> and <literal>node2</literal> where
> +     <literal>t1</literal> data is present. This needs similar steps; the only
>
> SUGGESTION
> The following steps demonstrate adding a new primary (node3) that has
> no t1 data to the existing primaries (node1 and node2) where t1 data
> is present.

Modified

> ~~~
>
> 9. Adding a new node when table data is present on the new node
>
> SUGGESTION (title)
> Adding a new primary that has existing table data

Modified

> ~~~
>
> 10.
> +   <note>
> +    <para>
> +     Adding a new node when table data is present on the new node is not
> +     supported.
> +    </para>
> +   </note>
>
> SUGGESTION
> Adding a new primary that has existing table data is not supported.

Modified

> ~~~
>
> 11. Generic steps for adding a new node to an existing set of primaries
>
> SUGGESTION (title)
> Generic steps for adding a new primary to an existing set of primaries

Modified

Thanks for the comments, the v39 patch shared at [1] has the changes
for the same.
[1] - https://www.postgresql.org/message-id/CALDaNm2POATc_jwQ-8MBJgGCVZGdUNhnTv8zkBuGzLaY03dM%3DA%40mail.gmail.com

Regards,
Vignesh