| From: | "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com> |
|---|---|
| To: | "Takamichi Osumi (Fujitsu)" <osumi(dot)takamichi(at)fujitsu(dot)com>, 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, "amit(dot)kapila16(at)gmail(dot)com" <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "smithpb2250(at)gmail(dot)com" <smithpb2250(at)gmail(dot)com>, "vignesh21(at)gmail(dot)com" <vignesh21(at)gmail(dot)com>, "shveta(dot)malik(at)gmail(dot)com" <shveta(dot)malik(at)gmail(dot)com>, "dilipbalaut(at)gmail(dot)com" <dilipbalaut(at)gmail(dot)com>, "euler(at)eulerto(dot)com" <euler(at)eulerto(dot)com>, "m(dot)melihmutlu(at)gmail(dot)com" <m(dot)melihmutlu(at)gmail(dot)com>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, "marcos(at)f10(dot)com(dot)br" <marcos(at)f10(dot)com(dot)br>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | RE: Time delayed LR (WAS Re: logical replication restrictions) |
| Date: | 2023-02-03 09:42:34 |
| Message-ID: | OS3PR01MB62757B9F653FD23C7DCCC81D9ED79@OS3PR01MB6275.jpnprd01.prod.outlook.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thurs, Feb 2, 2023 16:04 PM Takamichi Osumi (Fujitsu) <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> Attached the updated patch v26 accordingly.
Thanks for your patch.
Here is a comment:
1. The checks in function AlterSubscription
+ /*
+ * The combination of parallel streaming mode and
+ * min_apply_delay is not allowed. See
+ * parse_subscription_options for details of the reason.
+ */
+ if (opts.streaming == LOGICALREP_STREAM_PARALLEL)
+ if ((IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY) && opts.min_apply_delay > 0) ||
+ (!IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY) && sub->minapplydelay > 0))
and
+ /*
+ * The combination of parallel streaming mode and
+ * min_apply_delay is not allowed.
+ */
+ if (opts.min_apply_delay > 0)
+ if ((IsSet(opts.specified_opts, SUBOPT_STREAMING) && opts.streaming == LOGICALREP_STREAM_PARALLEL) ||
+ (!IsSet(opts.specified_opts, SUBOPT_STREAMING) && sub->stream == LOGICALREP_STREAM_PARALLEL))
I think the case where the options "min_apply_delay>0" and "streaming=parallel"
are set at the same time seems to have been checked in the function
parse_subscription_options, how about simplifying these two if-statements here
to the following:
```
if (opts.streaming == LOGICALREP_STREAM_PARALLEL &&
!IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY) &&
sub->minapplydelay > 0)
and
if (opts.min_apply_delay > 0 &&
!IsSet(opts.specified_opts, SUBOPT_STREAMING) &&
sub->stream == LOGICALREP_STREAM_PARALLEL)
```
Regards,
Wang Wei
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2023-02-03 09:44:17 | Re: Perform streaming logical transactions by background workers and parallel apply |
| Previous Message | Andres Freund | 2023-02-03 09:24:18 | Re: Weird failure with latches in curculio on v15 |