Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)

> >
> > Ok. I agree that it is difficult to interpret it correctly. So even if
> > say that a new checkpoint has been explicitly requested, the user may
> > not understand that it affects current checkpoint behaviour unless the
> > user knows the internals of the checkpoint. How about naming the field
> > to 'throttled' (Yes/ No) since our objective is to show that the
> > current checkpoint is throttled or not.
>
> -1
>
> That "throttled" flag should be the same as having or not a "force" in the
> flags. We should be consistent and report information the same way, so either
> a lot of flags (is_throttled, is_force...) or as now a single field containing
> the set flags, so the current approach seems better. Also, it wouldn't be much
> better to show the checkpoint as not having the force flags and still not being
> throttled.

I think your understanding is wrong here. The flag which affects
throttling behaviour is CHECKPOINT_IMMEDIATE. 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.

> CHECKPOINT_REQUESTED will always be set by RequestCheckpoint(), and can be used
> to detect that someone wants a new checkpoint afterwards, whatever it's and
> whether or not the current checkpoint to be finished quickly. For this flag I
> think it's better to not report it in the view flags but with a new field, as
> discussed before, as it's really what it means.

I understand your suggestion of adding a new field to indicate whether
any of the new requests have been made or not. You just want this
field to represent only a new request or does it also represent the
current checkpoint to finish quickly.

> CHECKPOINT_IMMEDIATE is the only new flag that can be used in an already in
> progress checkpoint, so it can be simply added to the view flags.

As discussed upthread this is not advisable to do so. The content of
'flags' remains the same through the checkpoint. We cannot add a new
checkpoint's flag (CHECKPOINT_IMMEDIATE ) to the current one even
though it affects current checkpoint behaviour. Only thing we can do
is to add a new field to show that the current checkpoint is affected
with new requests.

> Why not just reporting (ckpt_flags & (CHECKPOINT_REQUESTED |
> CHECKPOINT_IMMEDIATE)) in the path(s) that can update the new flags for the
> view?

Where do you want to add this in the path?

I feel the new field name is confusing here.
'next_flags' - It shows all the flag values of the next checkpoint.
Based on this user can get to know that the new request has been made
and also if CHECKPOINT_IMMEDIATE is enabled here, then it indicates
that the current checkpoint also gets affected. You are not ok to use
this name as it confuses the user.
'throttled' - The value will be set to Yes/No based on the
CHECKPOINT_IMMEDIATE bit set in the new checkpoint request's flag.
This says that the current checkpoint is affected and also I thought
this is an indication that new requests have been made. But there is a
confusion here too. If the current checkpoint starts with
CHECKPOINT_IMMEDIATE which is described by the 'flags' field and there
is no new request, then the value of this field is 'Yes' (Not
throttling) which again confuses the user.
'new request' - The value will be set to Yes/No if any new checkpoint
requests. This just indicates whether new requests have been made or
not. It can not be used to infer other information.

Thought?

Thanks & Regards,
Nitin Jadhav

On Fri, Mar 11, 2022 at 3:34 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> On Fri, Mar 11, 2022 at 02:41:23PM +0530, Nitin Jadhav wrote:
> >
> > Ok. I agree that it is difficult to interpret it correctly. So even if
> > say that a new checkpoint has been explicitly requested, the user may
> > not understand that it affects current checkpoint behaviour unless the
> > user knows the internals of the checkpoint. How about naming the field
> > to 'throttled' (Yes/ No) since our objective is to show that the
> > current checkpoint is throttled or not.
>
> -1
>
> That "throttled" flag should be the same as having or not a "force" in the
> flags. We should be consistent and report information the same way, so either
> a lot of flags (is_throttled, is_force...) or as now a single field containing
> the set flags, so the current approach seems better. Also, it wouldn't be much
> better to show the checkpoint as not having the force flags and still not being
> throttled.
>
> Why not just reporting (ckpt_flags & (CHECKPOINT_REQUESTED |
> CHECKPOINT_IMMEDIATE)) in the path(s) that can update the new flags for the
> view?
>
> CHECKPOINT_REQUESTED will always be set by RequestCheckpoint(), and can be used
> to detect that someone wants a new checkpoint afterwards, whatever it's and
> whether or not the current checkpoint to be finished quickly. For this flag I
> think it's better to not report it in the view flags but with a new field, as
> discussed before, as it's really what it means.
>
> CHECKPOINT_IMMEDIATE is the only new flag that can be used in an already in
> progress checkpoint, so it can be simply added to the view flags.