On Wed, Nov 9, 2022 at 12:11 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Wed, 10 Aug 2022 17:33:00 -0300, "Euler Taveira" <euler@eulerto.com> wrote in
> > On Wed, Aug 10, 2022, at 9:39 AM, osumi.takamichi@fujitsu.com wrote:
> > > Minor review comments for v6.
> > Thanks for your review. I'm attaching v7.
>
> Using interval is not standard as this kind of parameters but it seems
> convenient. On the other hand, it's not great that the unit month
> introduces some subtle ambiguity. This patch translates a month to 30
> days but I'm not sure it's the right thing to do. Perhaps we shouldn't
> allow the units upper than days.
>
Agreed. Isn't the same thing already apply to recovery_min_apply_delay
for which the maximum unit seems to be in days? If so, there is no
reason to do something different here?
> apply_delay() chokes the message-receiving path so that a not-so-long
> delay can cause a replication timeout to fire. I think we should
> process walsender pings even while delaying. Needing to make
> replication timeout longer than apply delay is not great, I think.
>
Again, I think for this case also the behavior should be similar to
how we handle recovery_min_apply_delay.
--
With Regards,
Amit Kapila.
