public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Takamichi Osumi (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]>
Subject: Re: Time delayed LR (WAS Re: logical replication restrictions)
Date: Wed, 22 Feb 2023 17:25:32 +0530
Message-ID: <CAA4eK1++pGobxhw6WiAGpuiQMr_cQ2NiHvMq1inn-YV54Cx6fg@mail.gmail.com> (raw)
In-Reply-To: <TYAPR01MB5866C6BCA4D9386D9C486033F5A59@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <TYAPR01MB5866A9A39100EDA85E612B3EF5A39@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB5866A2D600A86F743DDF8FE5F5A09@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB5866F00191375D0193320A4DF5A19@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<CAA4eK1K4uPbudrNdH+=_vN-Hpe9wYh=3vBS5Ww9dHn-LOWMV0g@mail.gmail.com>
<TYAPR01MB5866504C73ACCF98E0E09B11F5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<CAHut+Pu6Y+BkYKg6MYGi2zGnx6imeK4QzxBVhpQoPMeDr1npnQ@mail.gmail.com>
<TYAPR01MB5866C6BCA4D9386D9C486033F5A59@TYAPR01MB5866.jpnprd01.prod.outlook.com>
On Tue, Feb 21, 2023 at 1:28 PM Hayato Kuroda (Fujitsu)
<[email protected]> wrote:
>
> > doc/src/sgml/catalogs.sgml
> >
> > 4.
> > + <row>
> > + <entry role="catalog_table_entry"><para role="column_definition">
> > + <structfield>subminsenddelay</structfield> <type>int4</type>
> > + </para>
> > + <para>
> > + The minimum delay, in milliseconds, by the publisher to send changes
> > + </para></entry>
> > + </row>
> >
> > "by the publisher to send changes" --> "by the publisher before sending changes"
>
> As Amit said[1], there is a possibility to delay after sending delay. So I changed to
> "before sending COMMIT record". How do you think?
>
I think it would be better to say: "The minimum delay, in
milliseconds, by the publisher before sending all the changes". If you
agree then similar change is required in below comment as well:
+ /*
+ * The minimum delay, in milliseconds, by the publisher before sending
+ * COMMIT/PREPARE record.
+ */
+ int32 min_send_delay;
+
>
> > src/backend/replication/pgoutput/pgoutput.c
> >
> > 11.
> > + errno = 0;
> > + parsed = strtoul(strVal(defel->arg), &endptr, 10);
> > + if (errno != 0 || *endptr != '\0')
> > + ereport(ERROR,
> > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > + errmsg("invalid min_send_delay")));
> > +
> > + if (parsed > PG_INT32_MAX)
> > + ereport(ERROR,
> > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > + errmsg("min_send_delay \"%s\" out of range",
> > + strVal(defel->arg))));
> >
> > Should the validation be also checking/asserting no negative numbers,
> > or actually should the min_send_delay be defined as a uint32 in the
> > first place?
>
> I think you are right because min_apply_delay does not have related code.
> we must consider additional possibility that user may send START_REPLICATION
> by hand and it has minus value.
> Fixed.
>
Your reasoning for adding the additional check seems good to me but I
don't see it in the patch. The check as I see is as below:
+ if (delay_val > PG_INT32_MAX)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("min_send_delay \"%s\" out of range",
+ strVal(defel->arg))));
Am, I missing something, and the new check is at some other place?
+ has been finished. However, there is a possibility that the table
+ status written in <link
linkend="catalog-pg-subscription-rel"><structname>pg_subscription_rel</structname></link>
+ will be delayed in getting to "ready" state, and also two-phase
+ (if specified) will be delayed in getting to "enabled".
+ </para>
There appears to be a special value <0x00> after "ready". I think that
is added by mistake or probably you have used some editor which has
added this value. Can we slightly reword this to: "However, there is a
possibility that the table status updated in <link
linkend="catalog-pg-subscription-rel"><structname>pg_subscription_rel</structname></link>
could be delayed in getting to the "ready" state, and also two-phase
(if specified) could be delayed in getting to "enabled"."?
--
With Regards,
Amit Kapila.
view thread (17+ 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], [email protected]
Subject: Re: Time delayed LR (WAS Re: logical replication restrictions)
In-Reply-To: <CAA4eK1++pGobxhw6WiAGpuiQMr_cQ2NiHvMq1inn-YV54Cx6fg@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