I was surprised to see that this patch was never committed!

On Mon, Jan 31, 2022 at 04:28:11PM +0900, Kyotaro Horiguchi wrote:
> +static void
> +WalRcvComputeNextWakeup(WalRcvInfo *state,
> +                                             WalRcvWakeupReason reason,
> +                                             TimestampTz now)
> +{
> +     switch (reason)
> +     {
> ...
> +     default:
> +             break;
> +     }
> +}
> 
> Mmm.. there's NUM_WALRCV_WAKEUPS.. But I think we don't need to allow
> the case.  Isn't it better to replace the break with Assert()?

+1

> It might be suitable for another patch, but we can do that
> together. If we passed "now" to the function XLogWalRcvProcessMsg, we
> would be able to remove further several calls to
> GetCurrentTimestamp(), in the function itself and
> ProcessWalSndrMessage (the name is improperly abbreviated..).

While reading the patch, I did find myself wondering if there were some
additional opportunities for reducing the calls to GetCurrentTimestamp().
That was really the only thing that stood out to me.

Thomas, are you planning to continue with this patch?  If not, I don't mind
picking it up.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com


Reply via email to