Re: Handle infinite recursion in logical replication setup

Hi,

On 7/21/22 6:34 AM, vignesh C wrote:
> On Thu, Jul 21, 2022 at 2:06 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Wed, Jul 20, 2022 at 2:33 PM vignesh C <vignesh21@gmail.com> wrote:
>>>
>>> Modified. Apart from this I have run pgperltidy on the perl file and
>>> renamed 032_origin.pl to 030_origin.pl as currently there is
>>> 029_on_error.pl, 031_column_list.pl and there is no 030_*****.pl file.
>>> Thanks for the comment, the attached patch has the changes for the same.
>>>
>>
>> Pushed. Kindly rebase the remaining patches.
> 
> Thanks for pushing the patch.
> The attached v37 version contains the rebased patch for the remaining patches.

Thanks for the work on this feature -- this is definitely very helpful 
towards supporting more types of use cases with logical replication!

I've read through the proposed documentation and did some light testing 
of the patch. I have two general comments about the docs as they 
currently read:

1. I'm concerned by calling this "Bidirectional replication" in the docs 
that we are overstating the current capabilities. I think this is 
accentuated int he opening paragraph:

==snip==
  Bidirectional replication is useful for creating a multi-master database
  environment for replicating read/write operations performed by any of the
  member nodes.
==snip==

For one, we're not replicating reads, we're replicating writes. Amongst 
the writes, at this point we're only replicating DML. A reader could 
think that deploying can work for a full bidirectional solution.

(Even if we're aspirationally calling this section "Bidirectional 
replication", that does make it sound like we're limited to two nodes, 
when we can support more than two).

Perhaps "Logical replication between writers" or "Logical replication 
between primaries" or "Replicating changes between primaries", or 
something better.

2. There is no mention of conflicts in the documentation, e.g. 
referencing the "Conflicts" section of the documentation. It's very easy 
to create a conflicting transaction that causes a subscriber to be 
unable to continue to apply transactions:

   -- DB 1
   CREATE TABLE abc (id int);
   CREATE PUBLICATION node1 FOR ALL TABLES ;

   -- DB2
   CREATE TABLE abc (id int);
   CREATE PUBLICATION node2 FOR ALL TABLES ;
   CREATE SUBSCRIPTION node2_node1
     CONNECTION 'dbname=logi port=5433'
     PUBLICATION node1
     WITH (copy_data = off, origin = none);

   -- DB1
   CREATE SUBSCRIPTION node1_node2
     CONNECTION 'dbname=logi port=5434'
     PUBLICATION node2
     WITH (copy_data = off, origin = none);
   INSERT INTO abc VALUES (1);

   -- DB2
   INSERT INTO abc VALUES (2);

   -- DB1
   ALTER TABLE abc ADD PRIMARY KEY id;
   INSERT INTO abc VALUES (3);

   -- DB2
   INSERT INTO abc VALUES (3);

   -- DB1 cannot apply the transactions

At a minimum, I think we should reference the documentation we have in 
the logical replication section on conflicts. We may also want to advise 
that a user is responsible for designing their schemas in a way to 
minimize the risk of conflicts.

Thanks,

Jonathan