Re: Deduplicate code updating ControleFile's DBState.

On Fri, Nov 26, 2021 at 2:48 PM Amul Sul <sulamul(at)gmail(dot)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.