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....@gmail.com> wrote: > > At Wed, 7 Dec 2022 11:28:23 +0530, Sravan Kumar <sravanvcyb...@gmail.com> > wrote in > > On Tue, Dec 6, 2022 at 5:24 PM Bharath Rupireddy < > > bharath.rupireddyforpostg...@gmail.com> wrote: > > > > > On Tue, Dec 6, 2022 at 4:57 PM Sravan Kumar <sravanvcyb...@gmail.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
