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

On Wed, Jul 27, 2022 at 10:06 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Jul 26, 2022 at 2:30 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Fri, Jul 22, 2022 at 8:27 AM wangw.fnst@fujitsu.com
> > <wangw.fnst@fujitsu.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?


[1]
+apply_bgworker_relation_check(LogicalRepRelMapEntry *rel)
+{
+ /* Skip check if not an apply background worker. */
+ if (!am_apply_bgworker())
+ return;
+
+ /*
+ * Partition table checks are done later in function
+ * apply_handle_tuple_routing.
+ */
+ if (rel->localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ return;
+
+ /*
+ * Return if changes on this relation can be applied by an apply background
+ * worker.
+ */
+ if (rel->parallel_apply == PARALLEL_APPLY_SAFE)
+ return;
+
+ /* We are in error mode and should give user correct error. */
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot replicate target relation \"%s.%s\" using "
+ "subscription parameter streaming=parallel",
+ rel->remoterel.nspname, rel->remoterel.relname),
+ errdetail("The unique column on subscriber is not the unique "
+    "column on publisher or there is at least one "
+    "non-immutable function."),
+ errhint("Please change to use subscription parameter "
+ "streaming=on.")));
+}

[2]
> > +    /* Check the foreign keys. */
> > +    fkeys = RelationGetFKeyList(entry->localrel);
> > +    if (fkeys)
> > +        entry->parallel_apply = PARALLEL_APPLY_UNSAFE;

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