Re: Handle infinite recursion in logical replication setup
vignesh C <vignesh21@gmail.com>
From: vignesh C <vignesh21@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>,
Amit Kapila <amit.kapila16@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-06-01T17:57:36Z
Lists: pgsql-hackers
On Fri, May 27, 2022 at 2:08 PM Peter Smith <smithpb2250@gmail.com> wrote: > > On Fri, May 27, 2022 at 5:04 PM shiy.fnst@fujitsu.com > <shiy.fnst@fujitsu.com> wrote: > > > > On Wed, May 25, 2022 7:55 PM vignesh C <vignesh21@gmail.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. Yes, the generic steps are correct. modified > ~~~ > > 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) In generic steps we mention the publication must be created in a new node and we don't say about the existing nodes, because the publication would be existing already. I feel the existing order of publication creation in the TWO-node case (section 31.11.1) is ok. Thanks for the comments, the v17 patch attached at [1] has the changes for the same. [1] - https://www.postgresql.org/message-id/CALDaNm1rMihO7daiFyLdxkqArrC%2BdtM61oPXc-XrTYBYnJg3nw%40mail.gmail.com Regards, Vignesh
Commits
-
Improved CREATE SUBSCRIPTION message for clarity
- 1e7ca1189ccc 17.0 landed
- 842b65905046 16.0 landed
-
Fix the test case introduced by commit 8756930190.
- 032465157347 16.0 landed
-
Raise a warning if there is a possibility of data from multiple origins.
- 875693019053 16.0 landed
-
Add wait_for_subscription_sync for TAP tests.
- 0c20dd33db16 16.0 cited
-
Bump catversion for commit d8cd0c6c95c0120168df93aae095df4e0682a08a.
- 5f858dd3bebd 16.0 cited
-
Allow users to skip logical replication of data having origin.
- 366283961ac0 16.0 landed
-
Improve two comments related to a boolean DefElem's value
- 8445f5a21d40 16.0 cited
-
doc: Fix man page whitespace issues
- d7ab2a9a3c0a 15.0 cited