Re: Handle infinite recursion in logical replication setup

On Fri, May 27, 2022 at 5:04 PM shiy(dot)fnst(at)fujitsu(dot)com
<shiy(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Wed, May 25, 2022 7:55 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > The attached v16 patch has the changes for the same.
> >
>
> Thanks for updating the patch.
>
> Some comments for the document in 0002 patch.
>
> 1.
> + <para>
> + Lock the required tables in <literal>node1</literal> and
> + <literal>node2</literal> till the setup is completed.
> + </para>
> +
> + <para>
> + Create a publication in <literal>node1</literal>:
> +<programlisting>
> +node1=# CREATE PUBLICATION pub_node1 FOR TABLE t1;
> +CREATE PUBLICATION
> +</programlisting></para>
>
> If the table is locked in the very beginning, we will not be able to create the
> publication (because the locks have conflict). Maybe we should switch the order
> of creating publication and locking tables here.
>

I agree. It seems some of the locking instructions in the earlier
sections 31.11.1 - 31.11.4 are contradictory to the later "generic"
steps given in "31.11.5. Generic steps to add a new node to the
existing set of nodes". I'm assuming the generic steps are the
"correct" steps

e.g. generic steps say get the lock on new node tables AFTER the
publication of new node.
e.g. generic steps say do NOT get a table lock on the node one you are
(first) joining to.

~~~

Furthermore, the generic steps are describing attaching a new N+1th
node to some (1 ... N) other nodes.

So I think in the TWO-node case (section 31.11.1) node2 should be
treated as the "new" node that you are attaching to the "first" node1.
IMO the section 31.11.1 example should be reversed so that it obeys
the "generic" pattern.
e.g. It should be doing the CREATE PUBLICATION pub_node2 first (since
that is the "new" node)

------
Kind Regards,
Peter Smith.
Fujitsu Australia