Re: Handle infinite recursion in logical replication setup

On Tue, Jul 26, 2022 at 7:16 AM Peter Smith <smithpb2250(at)gmail(dot)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