public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Takamichi Osumi (Fujitsu) <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: vignesh C <[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]>
Subject: Re: Time delayed LR (WAS Re: logical replication restrictions)
Date: Mon, 23 Jan 2023 17:36:13 +0530
Message-ID: <CAA4eK1JK+3hO2Ki4h2bkZazyKHtTzvD77V5DZqFdTUNDtdocvQ@mail.gmail.com> (raw)
In-Reply-To: <TYCPR01MB8373A23EFC25B22CB132383AEDCB9@TYCPR01MB8373.jpnprd01.prod.outlook.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>
<CAHut+Puwrb_DEV_7ANSKrQr23-dAmL9vrNV8ww7XsAP+kej0uw@mail.gmail.com>
<TYCPR01MB8373BED9E390C4839AF56685EDC59@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<TYCPR01MB8373A23EFC25B22CB132383AEDCB9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
On Sun, Jan 22, 2023 at 6:12 PM Takamichi Osumi (Fujitsu)
<[email protected]> wrote:
>
>
> Attached the updated patch v19.
>
Few comments:
=============
1.
}
+
+
+/*
Only one empty line is sufficient between different functions.
2.
+ if (IsSet(supported_opts, SUBOPT_MIN_APPLY_DELAY) &&
+ opts->min_apply_delay > 0 && 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"));
}
I think here we should add a comment for the translator as we are
doing in some other nearby cases.
3.
+ /*
+ * The combination of parallel streaming mode and
+ * min_apply_delay is not allowed.
+ */
+ 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))
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot enable %s mode for subscription with %s",
+ "streaming = parallel", "min_apply_delay"));
+
A. When can second condition ((!IsSet(opts.specified_opts,
SUBOPT_MIN_APPLY_DELAY) && sub->minapplydelay > 0)) in above check be
true?
B. In comments, you can say "See parse_subscription_options."
4.
+/*
+ * When min_apply_delay parameter is set on the subscriber, we wait long enough
+ * to make sure a transaction is applied at least that interval behind the
+ * publisher.
Shouldn't this part of the comment needs to be updated after the patch
has stopped using interval?
5. How does this feature interacts with the SKIP feature? Currently,
it doesn't care whether the changes of a particular xact are skipped
or not. I think that might be okay because anyway the purpose of this
feature is to make subscriber lag from publishers. What do you think?
I feel we can add some comments to indicate the same.
--
With Regards,
Amit Kapila.
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: <CAA4eK1JK+3hO2Ki4h2bkZazyKHtTzvD77V5DZqFdTUNDtdocvQ@mail.gmail.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