Re: Handle infinite recursion in logical replication setup

On Mon, 29 Aug 2022 at 12:01, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Here are some review comments for patch v42-0002:
>
> ======
>
> 1. doc/src/sgml/logical-replication.sgml
>
> <literal>copy_data = true </literal>
>
> There are a couple of these tags where there is a trailing space
> before the </literal>. I guess it is doing no harm, but it is doing no
> good either, so IMO better to get rid of the space.

Modified

> ~~
>
> 2. doc/src/sgml/logical-replication.sgml - 31.3.1. Setting replication
> between two primaries
>
> User need to ensure that no data changes happen on table t1 on
> primary1 and primary2 until the setup is completed.
>
> SUGGESTION
> The user must ensure that no data changes happen on table t1 of
> primary1 and primary2 until the setup is completed.

Modified

> ~~~
>
> 3. doc/src/sgml/logical-replication.sgml - 31.3.2. Adding a new
> primary when there is no table data on any of the primaries
>
> User need to ensure that no data changes happen on table t1 on all the
> primaries primary1, primary2 and primary3 until the setup is
> completed.
>
> SUGGESTION
> The user must ensure that no data changes happen on table t1 of all
> the primaries primary1, primary2 and primary3 until the setup is
> completed.

Modified

> ~~~
>
> 4. doc/src/sgml/logical-replication.sgml - 31.3.3. Adding a new
> primary when table data is present on the existing primaries
>
> User need to ensure that no operations should be performed on table t1
> on primary2 and primary3 until the setup is completed.
>
> SUGGESTION
> The user must ensure that no operations are performed on table t1 of
> primary2 and primary3 until the setup is completed.
>
> Here also you changed the wording - "no data changes happen" versus
> "no operations should be performed". Was that a deliberate difference?
> Maybe they should all be consistent wording? If they are
> interchangeable IMO the "no operations are performed..." wording was
> the better of the two.

Modified, it was not a deliberate change. I have changed it to "no
operations are performed" in other places too.

> ~~~
>
> 5. doc/src/sgml/logical-replication.sgml - 31.3.4. Generic steps for
> adding a new primary to an existing set of primaries
>
> Step-2: User need to ensure that no changes happen on the required
> tables of the new primary until the setup is complete.
>
> SUGGESTION
> Step-2: The user must ensure that no changes happen on the required
> tables of the new primary until the setup is complete.

Modified

> ~~~
>
> 6.
>
> Step-4. User need to ensure that no changes happen on the required
> tables of the existing primaries except the first primary until the
> setup is complete.
>
> SUGGESTION
> Step-4. The user must ensure that no changes happen on the required
> tables of the existing primaries except the first primary until the
> setup is complete.

Modified

> ~~~
>
> 7. (the example)
>
> 7a.
> Step-2. User need to ensure that no changes happen on table t1 on
> primary4 until the setup is completed.
>
> SUGGESTION
> Step-2. The user must ensure that no changes happen on table t1 of
> primary4 until the setup is completed.

Modified

> ~
>
> 7b.
> Step-4. User need to ensure that no changes happen on table t1 on
> primary2 and primary3 until the setup is completed.
>
> SUGGESTION
> Step-4. The user must ensure that no changes happen on table t1 of
> primary2 and primary3 until the setup is completed.

Modified

Thanks for the comments, the changes for the same are available in the
v43 patch attached at [1].
[1] - https://www.postgresql.org/message-id/CALDaNm1V%2BAvzPsUcq%3DmNYG%2BJfAyovTWBe4vWKTknOgH_ko1E0Q%40mail.gmail.com

Regards,
Vignesh