Re: Suppressing useless wakeups in walreceiver

On Sun, Oct 16, 2022 at 9:29 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> Here's a different take. Instead of creating structs and altering function
> signatures, we can just make the wake-up times file-global, and we can skip
> the changes related to reducing the number of calls to
> GetCurrentTimestamp() for now. This results in a far less invasive patch.
> (I still think reducing the number of calls to GetCurrentTimestamp() is
> worthwhile, but I'm beginning to agree with Bharath that it should be
> handled separately.)
>
> Thoughts?

Thanks. I'm wondering if we need to extend the similar approach for
logical replication workers in logical/worker.c LogicalRepApplyLoop()?

A comment on the patch:
Isn't it better we track the soonest wakeup time or min of wakeup[]
array (update the min whenever the array element is updated in
WalRcvComputeNextWakeup()) instead of recomputing everytime by looping
for NUM_WALRCV_WAKEUPS times? I think this will save us some CPU
cycles, because the for loop, in which the below code is placed, runs
till the walreceiver end of life cycle. We may wrap wakeup[] and min
inside a structure for better code organization or just add min as
another static variable alongside wakeup[].

+ /* Find the soonest wakeup time, to limit our nap. */
+ nextWakeup = PG_INT64_MAX;
+ for (int i = 0; i < NUM_WALRCV_WAKEUPS; ++i)
+ nextWakeup = Min(wakeup[i], nextWakeup);
+ nap = Max(0, (nextWakeup - now + 999) / 1000);

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com