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

From: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
To: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Magnus Hagander <magnus(at)hagander(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)
Date: 2022-03-11 08:47:59
Message-ID: CAMm1aWavXxAzgi=HpT_fYs+9Tei-dSa9tjby0vJGcwiBgWwWRg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> > The current format matches with the server log message for the
> > checkpoint start in LogCheckpointStart(). Just to be consistent, I
> > have not changed the code.
> >
>
> See below, how flags are shown in other sql functions like:
>
> ashu(at)postgres=# select * from heap_tuple_infomask_flags(2304, 1);
> raw_flags | combined_flags
> -----------------------------------------+----------------
> {HEAP_XMIN_COMMITTED,HEAP_XMAX_INVALID} | {}
> (1 row)
>
> This looks more readable and it's easy to understand for the
> end-users.. Further comparing the way log messages are displayed with
> the way sql functions display its output doesn't look like a right
> comparison to me. Obviously both should show matching data but the way
> it is shown doesn't need to be the same. In fact it is not in most of
> the cases.

ok. I will take care in the next patch. I would like to handle this at
the SQL level in system_views.sql. The following can be used to
display in the format described above.

( '{' ||
CASE WHEN (S.param2 & 4) > 0 THEN 'immediate' ELSE '' END ||
CASE WHEN (S.param2 & 4) > 0 AND (S.param2 & -8) > 0 THEN ',
' ELSE '' END ||
CASE WHEN (S.param2 & 8) > 0 THEN 'force' ELSE '' END ||
CASE WHEN (S.param2 & 8) > 0 AND (S.param2 & -16) > 0 THEN
', ' ELSE '' END ||
CASE WHEN (S.param2 & 16) > 0 THEN 'flush-all' ELSE '' END ||
CASE WHEN (S.param2 & 16) > 0 AND (S.param2 & -32) > 0 THEN
', ' ELSE '' END ||
CASE WHEN (S.param2 & 32) > 0 THEN 'wait' ELSE '' END ||
CASE WHEN (S.param2 & 32) > 0 AND (S.param2 & -128) > 0 THEN
', ' ELSE '' END ||
CASE WHEN (S.param2 & 128) > 0 THEN 'wal' ELSE '' END ||
CASE WHEN (S.param2 & 128) > 0 AND (S.param2 & -256) > 0
THEN ', ' ELSE '' END ||
CASE WHEN (S.param2 & 256) > 0 THEN 'time' ELSE '' END
|| '}'

Basically, a separate CASE statement is used to decide whether a comma
has to be printed or not, which is done by checking whether the
previous flag bit is enabled (so that the appropriate flag has to be
displayed) and if any next bits are enabled (So there are some more
flags to be displayed). Kindly let me know if you know any other
better approach.

Thanks & Regards,
Nitin Jadhav

On Wed, Mar 9, 2022 at 7:07 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>
> On Tue, Mar 8, 2022 at 8:31 PM Nitin Jadhav
> <nitinjadhavpostgres(at)gmail(dot)com> wrote:
> >
> > > > [local]:5432 ashu(at)postgres=# select * from pg_stat_progress_checkpoint;
> > > > -[ RECORD 1 ]-----+-------------------------------------
> > > > pid | 22043
> > > > type | checkpoint
> > > > kind | immediate force wait requested time
> > > >
> > > > I think the output in the kind column can be displayed as {immediate,
> > > > force, wait, requested, time}. By the way these are all checkpoint
> > > > flags so it is better to display it as checkpoint flags instead of
> > > > checkpoint kind as mentioned in one of my previous comments.
> > >
> > > I will update in the next patch.
> >
> > The current format matches with the server log message for the
> > checkpoint start in LogCheckpointStart(). Just to be consistent, I
> > have not changed the code.
> >
>
> See below, how flags are shown in other sql functions like:
>
> ashu(at)postgres=# select * from heap_tuple_infomask_flags(2304, 1);
> raw_flags | combined_flags
> -----------------------------------------+----------------
> {HEAP_XMIN_COMMITTED,HEAP_XMAX_INVALID} | {}
> (1 row)
>
> This looks more readable and it's easy to understand for the
> end-users.. Further comparing the way log messages are displayed with
> the way sql functions display its output doesn't look like a right
> comparison to me. Obviously both should show matching data but the way
> it is shown doesn't need to be the same. In fact it is not in most of
> the cases.
>
> > I have taken care of the rest of the comments in v5 patch for which
> > there was clarity.
> >
>
> Thank you very much. Will take a look at it later.
>
> --
> With Regards,
> Ashutosh Sharma.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nitin Jadhav 2022-03-11 09:11:23 Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)
Previous Message Yura Sokolov 2022-03-11 08:30:27 Re: BufferAlloc: don't take two simultaneous locks