On Tue, Dec 6, 2022 at 1:30 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Here are some review comments for patch v9-0001:
>
> ======
>
> GENERAL
>
> 1. min_ prefix?
>
> What's the significance of the "min_" prefix for this parameter? I'm
> guessing the background is that at one time it was considered to be a
> GUC so took a name similar to GUC recovery_min_apply_delay (??)
>
> But in practice, I think it is meaningless and/or misleading. For
> example, suppose the user wants to defer replication by 1hr. IMO it is
> more natural to just say "defer replication by 1 hr" (aka
> apply_delay='1hr') Clearly it means replication will take place about
> 1 hr into the future. OTHO saying "defer replication by a MINIMUM of 1
> hr" (aka min_apply_delay='1hr') is quite vague because then it is
> equally valid if the replication gets delayed by 1 hr or 2 hrs or 5
> days or 3 weeks since all of those satisfy the minimum delay. The
> implementation could hardwire a delay of INT_MAX ms but clearly,
> that's not really what the user would expect.
>
There is another way to look at this naming. It is quite possible user
has set its value as '1 second' and the transaction is delayed by more
than that say because the publisher delayed sending it. There could be
various reasons why the publisher could delay like it was busy
processing another workload, the replication connection between
publisher and subscriber was not working, etc. Moreover, it will be
similar to the same parameter for physical replication. So, I think
keeping min in the name is a good idea.
--
With Regards,
Amit Kapila.
