On Mon, Aug 2, 2021 at 9:40 AM Amul Sul <sula...@gmail.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
