Horton

Re: Handle infinite recursion in logical replication setup

Dilip Kumar <dilipbalaut@gmail.com>

View thread View on postgr.es
From: Dilip Kumar <dilipbalaut@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.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-20T09:07:32Z
Lists: pgsql-hackers 
On Thu, Jun 16, 2022 at 3:48 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, Jun 15, 2022 at 12:09 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >

> Thanks for the comments, the attached v21 patch has the changes for the same.

I have done some basic review of v21 and I have a few comments,

1.
+/*
+ * The subscription will request the publisher to only send changes that
+ * originated locally.
+ */
+#define LOGICALREP_ORIGIN_LOCAL "local"
+
+/*
+ * The subscription will request the publisher to send any changes regardless
+ * of their origin
+ */
+#define LOGICALREP_ORIGIN_ANY "any"

Are we planning to extend this to more options or are we planning to
support the actual origin name here? If not then why isn't it just
bool?  I think the comments and the patch commit message should
explain the details behind it if it has been already discussed and
concluded.

2.
+/*
+ * Check and throw an error if the publisher has subscribed to the same table
+ * from some other publisher. This check is required only if copydata is ON and
+ * the origin is local.
+ */

I think it should also explain why this combination is not allowed and
if it is already explained in code
then this code can add comments to refer to that part of the code.


-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Commits

  1. Improved CREATE SUBSCRIPTION message for clarity

  2. Fix the test case introduced by commit 8756930190.

  3. Raise a warning if there is a possibility of data from multiple origins.

  4. Add wait_for_subscription_sync for TAP tests.

  5. Bump catversion for commit d8cd0c6c95c0120168df93aae095df4e0682a08a.

  6. Allow users to skip logical replication of data having origin.

  7. Improve two comments related to a boolean DefElem's value

  8. doc: Fix man page whitespace issues