On Thu, Nov 11, 2021 at 1:30 AM Bossart, Nathan <bossa...@amazon.com> wrote: > > On 10/1/21, 10:40 PM, "Michael Paquier" <mich...@paquier.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
v5-0001-Do-the-ControlFile-timestamp-setting-in-update_co.patch
Description: Binary data
v5-0002-Deduplicate-code-updating-ControleFile-s-DBState.patch
Description: Binary data