public inbox for [email protected]
help / color / mirror / Atom feedFrom: Takamichi Osumi (Fujitsu) <[email protected]>
To: 'vignesh C' <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: RE: Time delayed LR (WAS Re: logical replication restrictions)
Date: Fri, 20 Jan 2023 19:07:30 +0000
Message-ID: <TYCPR01MB83733699D782F9B1BD8E7399EDC59@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALDaNm3407AtkbvyL7X=Xxmyf7n4FMPQ+KTL-3deuYjKtf9K1Q@mail.gmail.com>
References: <TYCPR01MB837340F78F4A16F542589195EDFF9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CAJpy0uC2Czf2iJ17yHf4gnMb9pVJyc2QXVe6_NfxiwHhdNLmVg@mail.gmail.com>
<TYAPR01MB5866D0527B1B8D589F1C2551F5FC9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYCPR01MB83739C6133B50DDA8BAD1601EDFD9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CALDaNm2d=+WBGye015DoACY_PzrRKdmwjeZwDa+Y9LXFqnT0Ow@mail.gmail.com>
<TYCPR01MB8373F3E780D5BB60E441ECF3EDC69@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CAHut+PsydEoqQaDv861qS_366hBovYAE8RRoDu3=+sBM=yo3Dg@mail.gmail.com>
<CAHut+PtzoVdm7wj69fKDrh-KLvUHd4scdTunBsQLo97sZNFLGg@mail.gmail.com>
<TYCPR01MB8373F5162C7A0E6224670CF0EDC49@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CALDaNm3407AtkbvyL7X=Xxmyf7n4FMPQ+KTL-3deuYjKtf9K1Q@mail.gmail.com>
Hi,
On Thursday, January 19, 2023 7:55 PM vignesh C <[email protected]> wrote:
> On Thu, 19 Jan 2023 at 12:06, Takamichi Osumi (Fujitsu)
> <[email protected]> wrote:
> >
> > Updated the comment and the function call.
> >
> > Kindly have a look at the updated patch v17.
>
> Thanks for the updated patch, few comments:
> 1) min_apply_delay was accepting values like '600 m s h', I was not sure if we
> should allow this:
> alter subscription sub1 set (min_apply_delay = ' 600 m s h');
>
> + /*
> + * If no unit was specified, then explicitly
> add 'ms' otherwise
> + * the interval_in function would assume 'seconds'.
> + */
> + if (strspn(tmp, "-0123456789 ") == strlen(tmp))
> + val = psprintf("%sms", tmp);
> + else
> + val = tmp;
> +
> + interval =
> DatumGetIntervalP(DirectFunctionCall3(interval_in,
> +
>
> CStringGetDatum(val),
> +
>
> ObjectIdGetDatum(InvalidOid),
> +
> Int32GetDatum(-1)));
>
FYI, the input can be accepted by the interval type.
Now we changed the direction of the type from interval to integer
but plus some unit can be added like recovery_min_apply_delay.
Please check.
> 3) There is one check at parse_subscription_options and another check in
> AlterSubscription, this looks like a redundant check in case of alter
> subscription, can we try to merge and keep in one place:
> /*
> * The combination of parallel streaming mode and min_apply_delay is not
> * allowed.
> */
> if (IsSet(supported_opts, SUBOPT_MIN_APPLY_DELAY) &&
> opts->min_apply_delay > 0)
> {
> if (opts->streaming == LOGICALREP_STREAM_PARALLEL) ereport(ERROR,
> errcode(ERRCODE_SYNTAX_ERROR), errmsg("%s and %s are mutually
> exclusive options",
> "min_apply_delay > 0", "streaming = parallel")); }
>
> if (IsSet(opts.specified_opts, SUBOPT_MIN_APPLY_DELAY)) {
> /*
> * 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))
> ereport(ERROR,
> errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> errmsg("cannot enable %s for subscription in %s mode",
> "min_apply_delay", "streaming = parallel"));
>
> values[Anum_pg_subscription_subminapplydelay - 1] =
> Int64GetDatum(opts.min_apply_delay);
> replaces[Anum_pg_subscription_subminapplydelay - 1] = true; }
We can't. For create subscription, we need to check the patch
from parse_subscription_options, while for alter subscription,
we need to refer the current MySubscription value for those tests
in AlterSubscription.
> 4) typo "execeeds" should be "exceeds"
>
> + time on the subscriber. Any overhead of time spent in
> logical decoding
> + and in transferring the transaction may reduce the actual wait time.
> + It is also possible that the overhead already execeeds the
> requested
> + <literal>min_apply_delay</literal> value, in which case no
> additional
> + wait is necessary. If the system clocks on publisher and subscriber
> + are not synchronized, this may lead to apply changes earlier
> + than
Fixed.
Kindly have a look at the v18 patch in [1].
[1] - https://www.postgresql.org/message-id/TYCPR01MB8373BED9E390C4839AF56685EDC59%40TYCPR01MB8373.jpnprd0...
Best Regards,
Takamichi Osumi
view thread (40+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Time delayed LR (WAS Re: logical replication restrictions)
In-Reply-To: <TYCPR01MB83733699D782F9B1BD8E7399EDC59@TYCPR01MB8373.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox