Re: Deduplicate code updating ControleFile's DBState.

From: Amul Sul <sulamul(at)gmail(dot)com>
To: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Deduplicate code updating ControleFile's DBState.
Date: 2021-11-25 04:51:40
Message-ID: CAAJ_b96T5E3_jUCNDBVb-f08QyMTKL0+pJV-CK0QDkfX4OXujQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 11, 2021 at 1:30 AM Bossart, Nathan <bossartn(at)amazon(dot)com> wrote:
>
> On 10/1/21, 10:40 PM, "Michael Paquier" <michael(at)paquier(dot)xyz> wrote:
> > On Fri, Oct 01, 2021 at 05:47:45PM +0000, Bossart, Nathan wrote:
> >> I'm inclined to agree that anything that calls update_controlfile()
> >> should update the timestamp.
> >
> > pg_control.h tells that:
> > pg_time_t time; /* time stamp of last pg_control update */
> > So, yes, that would be more consistent.
> >
> >> However, I wonder if the additional
> >> calls to time() would have a noticeable impact.
> >
> > I would not take that lightly either. Now, I don't think that any of
> > the code paths where UpdateControlFile() or update_controlfile() is
> > called are hot enough to worry about that.
> >
> > UpdateControlFile(void)
> > {
> > + ControlFile->time = (pg_time_t) time(NULL);
> > update_controlfile(DataDir, ControlFile, true);
> > }
> > I have to admit that it is a bit strange to do that in the backend but
> > not the frontend, so there is a good argument for doing that directly
> > in update_controlfile(). pg_resetwal does an update of the time, but
> > pg_rewind does not.
>

Thanks for the inputs -- moved timestamp setting inside update_controlfile().

> I don't see any recent updates to this thread from Amul, so I'm
> marking this one as waiting-for-author.
>

Sorry for the delay, please have a look at the attached version --
changing status to Needs review, thanks.

Regards,
Amul

Attachment Content-Type Size
v5-0001-Do-the-ControlFile-timestamp-setting-in-update_co.patch application/octet-stream 4.4 KB
v5-0002-Deduplicate-code-updating-ControleFile-s-DBState.patch application/octet-stream 1.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2021-11-25 04:52:15 Re: pg_replslotdata - a tool for displaying replication slot information
Previous Message Amit Kapila 2021-11-25 04:21:22 Re: Non-superuser subscription owners