On Fri, Nov 26, 2021 at 2:48 PM Amul Sul <sula...@gmail.com> wrote: > > ControlFile.state = DB_SHUTDOWNED; > > - ControlFile.time = (pg_time_t) time(NULL); > > This one had better not be removed, either, as we require pg_resetwal > > to guess a set of control file values. Removing the one in > > RewriteControlFile() is fine, on the contrary. > > My bad, sorry for the sloppy change, corrected it in the attached version.
Thanks for the patch. By moving the time update to update_controlfile, the patch ensures that we have the correct last updated time. Earlier we were missing (in some places) to update the time before calling UpdateControlFile. Isn't it better if we update the ControlFile->time at the end of the update_controlfile, after file write/sync? Why do we even need UpdateControlFile which just calls another function? It may be there for usability and readability, but can't the pg backend code just call update_controlfile(DataDir, ControlFile, true); directly so that a function call cost can be avoided? Otherwise, why can't we make UpdateControlFile an inline function? I'm not sure if any of the compilers will ever optimize by inlining it without the "inline" keyword. Regards, Bharath Rupireddy.