From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
---|---|
To: | sravanvcybage(at)gmail(dot)com |
Cc: | bharath(dot)rupireddyforpostgres(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Question regarding "Make archiver process an auxiliary process. commit" |
Date: | 2022-12-07 06:19:39 |
Message-ID: | 20221207.151939.662411275254345514.horikyota.ntt@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2022-12-07 07:01:18 | Re: ExecRTCheckPerms() and many prunable partitions |
Previous Message | Kyotaro Horiguchi | 2022-12-07 06:01:02 | Re: Question regarding "Make archiver process an auxiliary process. commit" |