On Sun, Oct 16, 2022 at 9:29 AM Nathan Bossart <nathandboss...@gmail.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