Re: Question regarding "Make archiver process an auxiliary process. commit"

On Tue, Dec 6, 2022 at 5:24 PM Bharath Rupireddy <
bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:

> On Tue, Dec 6, 2022 at 4:57 PM Sravan Kumar <sravanvcybage(at)gmail(dot)com>
> wrote:
> >
> > Thank you for the feedback.
> >
> > I'll be glad to help with the fix. Here's the patch for review.
>
> Thanks. +1 for fixing this.
>> I would like to quote recent discussions on reducing the useless
>> wakeups or increasing the sleep/hibernation times in various processes
>> to reduce the power savings [1] [2] [3] [4] [5]. With that in context,
>> does the archiver need to wake up every 60 sec at all when its latch
>> gets set (PgArchWakeup()) whenever the server switches to a new WAL
>> file? What happens if we get rid of PGARCH_AUTOWAKE_INTERVAL and rely
>> on its latch being set? If required, we can spread PgArchWakeup() to
>> more places, no?
>
>
I like the idea of not having to wake up intermittently and probably we
should start a discussion about it.

I see the following comment in PgArchWakeup().

/*
* We don't acquire ProcArrayLock here. It's actually fine because
* procLatch isn't ever freed, so we just can potentially set the wrong
* process' (or no process') latch. Even in that case the archiver will
* be relaunched shortly and will start archiving.
*/

While I do not fully understand the comment, it gives me an impression that
the SetLatch() done here is counting on the timeout to wake up the archiver
in some cases where the latch is wrongly set.

The proposed idea is a behaviour change while this thread intends to clean
up some code that's
a result of the mentioned commit. So probably the proposed idea can be
discussed as a separate thread.

Before even answering the above questions, I think we need to see if
> there're any cases where a process can miss SetLatch() calls (I don't
> have an answer for that).
>
> Or do we want to stick to what the below comment says?
>
> /*
> * There shouldn't be anything for the archiver to do except to wait
> for a
> * signal ... however, the archiver exists to protect our data, so she
> * wakes up occasionally to allow herself to be proactive.
> */
>
> [1]
> https://www.postgresql.org/message-id/CA%2BhUKGJCbcv8AtujLw3kEO2wRB7Ffzo1fmwaGG-tQLuMOjf6qQ%40mail.gmail.com
> [2]
> commit cd4329d9393f84dce34f0bd2dd936adc8ffaa213
> Author: Thomas Munro <tmunro(at)postgresql(dot)org>
> Date: Tue Nov 29 11:28:08 2022 +1300
>
> Remove promote_trigger_file.
>
> Previously, an idle startup (recovery) process would wake up every 5
> seconds to have a chance to poll for promote_trigger_file, even if that
> GUC was not configured. That promotion triggering mechanism was
> effectively superseded by pg_ctl promote and pg_promote() a long time
> ago. There probably aren't many users left and it's very easy to
> change
> to the modern mechanisms, so we agreed to remove the feature.
>
> This is part of a campaign to reduce wakeups on idle systems.
>
> [3] https://commitfest.postgresql.org/41/4035/
> [4] https://commitfest.postgresql.org/41/4020/
> [5]
> commit 05a7be93558c614ab89c794cb1d301ea9ff33199
> Author: Thomas Munro <tmunro(at)postgresql(dot)org>
> Date: Tue Nov 8 20:36:36 2022 +1300
>
> Suppress useless wakeups in walreceiver.
>
> Instead of waking up 10 times per second to check for various timeout
> conditions, keep track of when we next have periodic work to do.
>
> --
> Bharath Rupireddy
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com
>

--
Thanks And Regards,
Sravan

Take life one day at a time.