Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs) - Mailing list pgsql-hackers

On Mon, Mar 14, 2022 at 03:16:50PM +0530, Nitin Jadhav wrote:
> > > I am not suggesting
> > > removing the existing 'flags' field of pg_stat_progress_checkpoint
> > > view and adding a new field 'throttled'. The content of the 'flags'
> > > field remains the same. I was suggesting replacing the 'next_flags'
> > > field with 'throttled' field since the new request with
> > > CHECKPOINT_IMMEDIATE flag enabled will affect the current checkpoint.
> >
> > Are you saying that this new throttled flag will only be set by the overloaded
> > flags in ckpt_flags?
>
> Yes. you are right.
>
> > So you can have a checkpoint with a CHECKPOINT_IMMEDIATE
> > flags that's throttled, and a checkpoint without the CHECKPOINT_IMMEDIATE flag
> > that's not throttled?
>
> I think it's the reverse. A checkpoint with a CHECKPOINT_IMMEDIATE
> flags that's not throttled (disables delays between writes) and  a
> checkpoint without the CHECKPOINT_IMMEDIATE flag that's throttled
> (enables delays between writes)

Yes that's how it's supposed to work, but my point was that your suggested
'throttled' flag could say the opposite, which is bad.

> > I don't get it.  The checkpoint flags and the view flags (set by
> > pgstat_progrss_update*) are different, so why can't we add this flag to the
> > view flags?  The fact that checkpointer.c doesn't update the passed flag and
> > instead look in the shmem to see if CHECKPOINT_IMMEDIATE has been set since is
> > an implementation detail, and the view shouldn't focus on which flags were
> > initially passed to the checkpointer but instead which flags the checkpointer
> > is actually enforcing, as that's what the user should be interested in.  If you
> > want to store it in another field internally but display it in the view with
> > the rest of the flags, I'm fine with it.
>
> Just to be in sync with the way code behaves, it is better not to
> update the next checkpoint request's CHECKPOINT_IMMEDIATE with the
> current checkpoint 'flags' field. Because the current checkpoint
> starts with a different set of flags and when there is a new request
> (with CHECKPOINT_IMMEDIATE), it just processes the pending operations
> quickly to take up next requests. If we update this information in the
> 'flags' field of the view, it says that the current checkpoint is
> started with CHECKPOINT_IMMEDIATE which is not true.

Which is why I suggested to only take into account CHECKPOINT_REQUESTED (to
be able to display that a new checkpoint was requested) and
CHECKPOINT_IMMEDIATE, to be able to display that the current checkpoint isn't
throttled anymore if it were.

I still don't understand why you want so much to display "how the checkpoint
was initially started" rather than "how the checkpoint is really behaving right
now".  The whole point of having a progress view is to have something dynamic
that reflects the current activity.

> Hence I had
> thought of adding a new field ('next flags' or 'upcoming flags') which
> contain all the flag values of new checkpoint requests. This field
> indicates whether the current checkpoint is throttled or not and also
> it indicates there are new requests.

I'm not opposed to having such a field, I'm opposed to having a view with "the
current checkpoint is throttled but if there are some flags in the next
checkpoint flags and those flags contain checkpoint immediate then the current
checkpoint isn't actually throttled anymore" behavior.