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

I have added the thread to the commitfest: https://commitfest.postgresql.org/42/
Did you get a chance to review the patch? Please let me know if you
need anything from my end.

Thanks & Regards,
Sravan Velagandula
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, Dec 7, 2022 at 11:49 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Wed, 7 Dec 2022 11:28:23 +0530, Sravan Kumar <sravanvcybage(at)gmail(dot)com> wrote in
> > 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.
>
> It is telling about the first iteration of archive process, not
> periodical wakeups. So I think it is doable by considering how to
> handle incomplete archiving iterations.
>
> > 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.
>
> Agreed.
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center

--
Thanks & Regards,
Sravan Velagandula
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company