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

From Matthias van de Meent
Subject Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)
Date
Msg-id CAEze2WiFSr-ALcNMg3VH6hUrXCnY=6+TLq0hpLCO59nuRwY3+Q@mail.gmail.com
Whole thread Raw
In response to Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)  (Julien Rouhaud <rjuju123@gmail.com>)
Responses Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)  (Julien Rouhaud <rjuju123@gmail.com>)
List pgsql-hackers
On Fri, 25 Feb 2022 at 17:35, Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Fri, Feb 25, 2022 at 08:53:50PM +0530, Nitin Jadhav wrote:
> > >
> > > I'm not sure what Matthias meant, but as far as I know there's no fundamental
> > > difference between checkpoint with and without the CHECKPOINT_IMMEDIATE flag,
> > > and there's also no scheduling for multiple checkpoints.
> > >
> > > Yes, the flags will remain the same but checkpoint.c will test both the passed
> > > flags and the shmem flags to see whether a delay should be added or not, which
> > > is the only difference in checkpoint processing for this flag.  See the call to
> > > ImmediateCheckpointRequested() which will look at the value in shmem:
> > >
> > >        /*
> > >         * Perform the usual duties and take a nap, unless we're behind schedule,
> > >         * in which case we just try to catch up as quickly as possible.
> > >         */
> > >        if (!(flags & CHECKPOINT_IMMEDIATE) &&
> > >                !ShutdownRequestPending &&
> > >                !ImmediateCheckpointRequested() &&
> > >                IsCheckpointOnSchedule(progress))
> >
> > I understand that the checkpointer considers flags as well as the
> > shmem flags and if CHECKPOINT_IMMEDIATE flag is set, it affects the
> > current checkpoint operation (No further delay) but does not change
> > the current flag value. Should we display this change in the kind
> > field of the view or not? Please share your thoughts.
>
> I think the fields should be added.  It's good to know that a checkpoint was
> trigger due to normal activity and should be spreaded, and then something
> upgraded it to an immediate checkpoint.  If you're desperately waiting for the
> end of a checkpoint for some reason and ask for an immediate checkpoint, you'll
> certainly be happy to see that the checkpointer is aware of it.
>
> But maybe I missed something in the code, so let's wait for Matthias input
> about it.

The point I was trying to make was "If cps->ckpt_flags is
CHECKPOINT_IMMEDIATE, we hurry up to start the new checkpoint that is
actually immediate". That doesn't mean that this checkpoint was
created with IMMEDIATE or running using IMMEDIATE, only that optional
delays are now being skipped instead.

To let the user detect _why_ the optional delays are now being
skipped, I propose not to report this currently running checkpoint's
"flags | CHECKPOINT_IMMEDIATE", but to add reporting of the next
checkpoint's flags; which would allow the detection and display of the
CHECKPOINT_IMMEDIATE we're actually hurrying for (plus some more
interesting information flags.

-Matthias

PS. I just noticed that the checkpoint flags are also being parsed and
stringified twice in LogCheckpointStart; and adding another duplicate
in the current code would put that at 3 copies of effectively the same
code. Do we maybe want to deduplicate that into macros, similar to
LSN_FORMAT_ARGS?



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Proposal: Support custom authentication methods using hooks
Next
From: Andres Freund
Date:
Subject: Re: convert libpq uri-regress tests to tap test