RE: Perform streaming logical transactions by background workers and parallel apply

On Wednesday, July 27, 2022 1:29 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Wed, Jul 27, 2022 at 10:06 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Tue, Jul 26, 2022 at 2:30 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Jul 22, 2022 at 8:27 AM wangw(dot)fnst(at)fujitsu(dot)com
> > > <wangw(dot)fnst(at)fujitsu(dot)com> wrote:
> > > >
> > > > On Tues, Jul 19, 2022 at 10:29 AM I wrote:
> > > > > Attach the news patches.
> > > >
> > > > Not able to apply patches cleanly because the change in HEAD
> (366283961a).
> > > > Therefore, I rebased the patch based on the changes in HEAD.
> > > >
> > > > Attach the new patches.
> > >
> > > + /* Check the foreign keys. */
> > > + fkeys = RelationGetFKeyList(entry->localrel);
> > > + if (fkeys)
> > > + entry->parallel_apply = PARALLEL_APPLY_UNSAFE;
> > >
> > > So if there is a foreign key on any of the tables which are parts of
> > > a subscription then we do not allow changes for that subscription to
> > > be applied in parallel?
> > >
> >
> > I think the above check will just prevent the parallelism for a xact
> > operating on the corresponding relation not the relations of the
> > entire subscription. Your statement sounds like you are saying that it
> > will prevent parallelism for all the other tables in the subscription
> > which has a table with FK.
>
> Okay, got it. I thought we are disallowing parallelism for the entire subscription.
>
> > > I think this is a big limitation because having foreign key on the
> > > table is very normal right? I agree that if we allow them then
> > > there could be failure due to out of order apply right?
> > >
> >
> > What kind of failure do you have in mind and how it can occur? The one
> > way it can fail is if the publisher doesn't have a corresponding
> > foreign key on the table because then the publisher could have allowed
> > an insert into a table (insert into FK table without having the
> > corresponding key in PK table) which may not be allowed on the
> > subscriber. However, I don't see any check that could prevent this
> > because for this we need to compare the FK list for a table from the
> > publisher with the corresponding one on the subscriber. I am not
> > really sure if due to the risk of such conflicts we should block
> > parallelism of transactions operating on tables with FK because those
> > conflicts can occur even without parallelism, it is just a matter of
> > timing. But, I could be missing something due to which the above check
> > can be useful?
>
> Actually, my question starts with this check[1][2], from this it
> appears that if this relation is having a foreign key then we are
> marking it parallel unsafe[2] and later in [1] while the worker is
> applying changes for that relation and if it was marked parallel
> unsafe then we are throwing error. So my question was why we are
> putting this restriction? Although this error is only talking about
> unique and non-immutable functions this is also giving an error if the
> target table had a foreign key. So my question was do we really need
> to restrict this? I mean why we are restricting this case?
>

Hi,

I think the foreign key check is used to prevent the apply worker from waiting
indefinitely which is caused by foreign key difference between publisher and
subscriber, Like the following example:

-------------------------------------
Publisher:
-- both table are published
CREATE TABLE PKTABLE ( ptest1 int);
CREATE TABLE FKTABLE ( ftest1 int);

-- initial data
INSERT INTO PKTABLE VALUES(1);

Subcriber:
CREATE TABLE PKTABLE ( ptest1 int PRIMARY KEY);
CREATE TABLE FKTABLE ( ftest1 int REFERENCES PKTABLE);

-- Execute the following transactions on publisher

Tx1:
INSERT ... -- make enough changes to start streaming mode
DELETE FROM PKTABLE;
Tx2:
INSERT ITNO FKTABLE VALUES(1);
COMMIT;
COMMIT;
-------------------------------------

The subcriber's apply worker will wait indefinitely, because the main apply worker is
waiting for the streaming transaction to finish which is in another apply
bgworker.

BTW, I think the foreign key won't take effect in subscriber's apply worker by
default. Because we set session_replication_role to 'replica' in apply worker
which prevent the FK trigger function to be executed(only the trigger with
FIRES_ON_REPLICA flag will be executed in this mode). User can only alter the
trigger to enable it on replica mode to make the foreign key work. So, ISTM, we
won't hit this ERROR frequently.

And based on this, another comment about the patch is that it seems unnecessary
to directly check the FK returned by RelationGetFKeyList. Checking the actual FK
trigger function seems enough.

Best regards,
Hou zj