public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Time delayed LR (WAS Re: logical replication restrictions)
Date: Mon, 30 Jan 2023 16:08:02 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA4eK1LZGUXRzUQzCiqjneZ+H_fenxPwefe6t_UxiZ=eBuW_Ew@mail.gmail.com>
References: <CAA4eK1K_iKpmhDg+gwpz9rjEauBRBtj-_vp1D0e6K6WP4DpsoQ@mail.gmail.com>
<[email protected]>
<CAA4eK1LZGUXRzUQzCiqjneZ+H_fenxPwefe6t_UxiZ=eBuW_Ew@mail.gmail.com>
At Mon, 30 Jan 2023 11:56:33 +0530, Amit Kapila <[email protected]> wrote in
> On Mon, Jan 30, 2023 at 9:43 AM Kyotaro Horiguchi
> <[email protected]> wrote:
> >
> > At Mon, 30 Jan 2023 08:51:05 +0530, Amit Kapila <[email protected]> wrote in
> > > On Mon, Jan 30, 2023 at 8:32 AM Kyotaro Horiguchi
> > > <[email protected]> wrote:
> > > >
> > > > At Sat, 28 Jan 2023 04:28:29 +0000, "Takamichi Osumi (Fujitsu)" <[email protected]> wrote in
> > > > > On Friday, January 27, 2023 8:00 PM Amit Kapila <[email protected]> wrote:
> > > > > > So, you have changed min_apply_delay from int64 to int32, but you haven't
> > > > > > mentioned the reason for the same? We use 'int' for the similar parameter
> > > > > > recovery_min_apply_delay, so, ideally, it makes sense but still better to tell your
> > > > > > reason explicitly.
> > > > > Yes. It's because I thought I need to make this feature consistent with the recovery_min_apply_delay.
> > > > > This feature handles the range same as the recovery_min_apply delay from 0 to INT_MAX now
> > > > > so should be adjusted to match it.
> > > >
> > > > INT_MAX can stick out of int32 on some platforms. (I'm not sure where
> > > > that actually happens, though.) We can use PG_INT32_MAX instead.
> > > >
> > >
> > > But in other integer GUCs including recovery_min_apply_delay, we use
> > > INT_MAX, so not sure if it is a good idea to do something different
> > > here.
> >
> > The GUC is not stored in a catalog, but.. oh... it is multiplied by
> > 1000.
>
> Which part of the patch you are referring to here? Isn't the check in
Where recovery_min_apply_delay is used. It is allowed to be set up to
INT_MAX but it is used as:
> delayUntil = TimestampTzPlusMilliseconds(xtime, recovery_min_apply_delay);
Where the macro is defined as:
> #define TimestampTzPlusMilliseconds(tz,ms) ((tz) + ((ms) * (int64) 1000))
Which can lead to overflow, which is practically harmless.
> the function defGetMinApplyDelay() sufficient to ensure that the
> 'delay' value stored in the catalog will always be lesser than
> INT_MAX?
I'm concerned about cases where INT_MAX is wider than int32. If we
don't assume such cases, I'm fine with INT_MAX there.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
view thread (2+ messages)
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: <[email protected]>
* 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