Re: Background writer and checkpointer in crash recovery

On Mon, Aug 2, 2021 at 9:40 AM Amul Sul <sulamul(at)gmail(dot)com> wrote:
> > That's great. I just realized that this leaves us with identical
> > RequestCheckpoint() calls in two nearby places. Is there any reason
> > not to further simplify as in the attached?
> >
> +1, also, can we just get rid off "promoted" flag? The only
> inconvenience is we might need to check three flags instead of one to
> perform the checkpoint at the end.

I'm not sure that's really a win, because if we use the same
conditional in both places then it might not be clear to somebody that
they're supposed to match.

I do think we ought to consider renaming the flag, though, because
LocalPromoteIsTriggered is already tracking whether we were promoted,
and what this flag is tracking is not quite that thing. It's real
purpose is to track whether we should request a non-immediate
checkpoint at the end of recovery, and I think we ought to name it
some way that reflects that, e.g. perform_checkpoint_later.

--
Robert Haas
EDB: http://www.enterprisedb.com