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

From: Julien Rouhaud <[email-masked]>
Date: Mon, 28 Feb 2022 20:58:58 +0800

On Mon, Feb 28, 2022 at 06:03:54PM +0530, Bharath Rupireddy wrote:
> On Mon, Feb 28, 2022 at 12:02 PM Julien Rouhaud <[email-masked]> wrote:
> >
> > I suggested upthread to store the starting timeline instead.  This way you can
> > deduce whether it's a restartpoint or a checkpoint, but you can also deduce
> > other information, like what was the starting WAL.
> 
> I don't understand why we need the timeline here to just determine
> whether it's a restartpoint or checkpoint.

I'm not saying it's necessary, I'm saying that for the same space usage we can
store something a bit more useful.  If no one cares about having the starting
timeline available for no extra cost then sure, let's just store the kind
directly.

> Can't the checkpoint start LSN be deduced from
> PROGRESS_CHECKPOINT_LSN, checkPoint.redo?

I'm not sure I'm following, isn't checkPoint.redo the checkpoint start LSN?

> > As mentioned upthread, there can be multiple backends that request a
> > checkpoint, so unless we want to store an array of pid we should store a number
> > of backend that are waiting for a new checkpoint.
> 
> Yeah, you are right. Let's not go that path and store an array of
> pids. I don't see a strong use-case with the pid of the process
> requesting checkpoint. If required, we can add it later once the
> pg_stat_progress_checkpoint view gets in.

I don't think that's really necessary to give the pid list.

If you requested a new checkpoint, it doesn't matter if it's only your backend
that triggered it, another backend or a few other dozen, the result will be the
same and you have the information that the request has been seen.  We could
store just a bool for that but having a number instead also gives a bit more
information and may allow you to detect some broken logic on your client code
if it keeps increasing.

---

From: Michael Paquier <[email-masked]>
Date: Tue, 5 Apr 2022 18:45:44 +0900

On Fri, Mar 18, 2022 at 05:15:56PM -0700, Andres Freund wrote:
> Have you measured the performance effects of this? On fast storage with large
> shared_buffers I've seen these loops in profiles. It's probably fine, but it'd
> be good to verify that.

I am wondering if we could make the function inlined at some point.
We could also play it safe and only update the counters every N loops
instead.

> This view is depressingly complicated. Added up the view definitions for
> the already existing pg_stat_progress* views add up to a measurable part of
> the size of an empty database:

Yeah.  I think that what's proposed could be simplified, and we had
better remove the fields that are not that useful.  First, do we have 
any need for next_flags?  Second, is the start LSN really necessary
for monitoring purposes?  Not all the information in the first
parameter is useful, as well.  For example "shutdown" will never be 
seen as it is not possible to use a session at this stage, no?  There
is also no gain in having "immediate", "flush-all", "force" and "wait"
(for this one if the checkpoint is requested the session doing the
work knows this information already).

A last thing is that we may gain in visibility by having more
attributes as an effect of splitting param2.  On thing that would make
sense is to track the reason why the checkpoint was triggered
separately (aka wal and time).  Should we use a text[] instead to list
all the parameters instead?  Using a space-separated list of items is
not intuitive IMO, and callers of this routine will likely parse
that.

Shouldn't we also track the number of files flushed in each sub-step?
In some deployments you could have a large number of 2PC files and
such.  We may want more information on such matters.

+                      WHEN 3 THEN 'checkpointing replication slots'
+                      WHEN 4 THEN 'checkpointing logical replication snapshot files'
+                      WHEN 5 THEN 'checkpointing logical rewrite mapping files'
+                      WHEN 6 THEN 'checkpointing replication origin'
+                      WHEN 7 THEN 'checkpointing commit log pages'
+                      WHEN 8 THEN 'checkpointing commit time stamp pages'
There is a lot of "checkpointing" here.  All those terms could be
shorter without losing their meaning.

This patch still needs some work, so I am marking it as RwF for now.
--
Michael

---

From: Nitin Jadhav <[email-masked]>
Date: Mon, 6 Jun 2022 11:33:50 +0530

Hi,

Here is the update patch which fixes the previous comments discussed
in this thread. I am sorry for the long gap in the discussion. Kindly
let me know if I have missed any of the comments or anything new.

Thanks & Regards,
Nitin Jadhav

On Fri, Mar 18, 2022 at 4:52 PM Nitin Jadhav
<[email-masked]> wrote:
>
> > > > 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)
>
> I will take care in the next patch.
>
> > > 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.
>
> I understand your point and I also agree that it becomes difficult for
> the user to understand the context.
>
> > 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.
>
> As of now I will not consider adding this information to the view. If
> required and nobody opposes having this included in the 'flags' field
> of the view, then I will consider adding.
>
> Thanks & Regards,
> Nitin Jadhav
>
> On Mon, Mar 14, 2022 at 5:16 PM Julien Rouhaud <[email-masked]> wrote:
> >
> > 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.

---

From: Nitin Jadhav <[email-masked]>
Date: Mon, 13 Jun 2022 19:08:35 +0530

> shared_buffers I've seen these loops in profiles. It's probably fine, but it'd

To understand the performance effects of the above, I have taken the
average of five checkpoints with the patch and without the patch in my
environment. Here are the results.
With patch: 269.65 s
Without patch: 269.60 s

It looks fine. Please share your views.

> the size of an empty database:

Thank you so much for sharing the detailed analysis. We can remove a
few fields which are not so important to make it simple.

Thanks & Regards,
Nitin Jadhav

On Sat, Mar 19, 2022 at 5:45 AM Andres Freund <[email-masked]> wrote:
>
> Hi,
>
> This is a long thread, sorry for asking if this has been asked before.
>
> On 2022-03-08 20:25:28 +0530, Nitin Jadhav wrote:
> >        * Sort buffers that need to be written to reduce the likelihood of random
> > @@ -2129,6 +2132,8 @@ BufferSync(int flags)
> >               bufHdr = GetBufferDescriptor(buf_id);
> >
> >               num_processed++;
> > +             pgstat_progress_update_param(PROGRESS_CHECKPOINT_BUFFERS_PROCESSED,
> > +                                                                      num_processed);
> >
> >               /*
> >                * We don't need to acquire the lock here, because we're only looking
> > @@ -2149,6 +2154,8 @@ BufferSync(int flags)
> >                               TRACE_POSTGRESQL_BUFFER_SYNC_WRITTEN(buf_id);
> >                               PendingCheckpointerStats.m_buf_written_checkpoints++;
> >                               num_written++;
> > +                             pgstat_progress_update_param(PROGRESS_CHECKPOINT_BUFFERS_WRITTEN,
> > +                                                                                      num_written);
> >                       }
> >               }
>
> > @@ -1897,6 +1897,112 @@ pg_stat_progress_basebackup| SELECT s.pid,
> >      s.param4 AS tablespaces_total,
> >      s.param5 AS tablespaces_streamed
> >     FROM pg_stat_get_progress_info('BASEBACKUP'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20);
> > +pg_stat_progress_checkpoint| SELECT s.pid,
> > +        CASE s.param1
> > +            WHEN 1 THEN 'checkpoint'::text
> > +            WHEN 2 THEN 'restartpoint'::text
> > +            ELSE NULL::text
> > +        END AS type,
> > +    (((((((
> > +        CASE
> > +            WHEN ((s.param2 & (1)::bigint) > 0) THEN 'shutdown '::text
> > +            ELSE ''::text
> > +        END ||
> > +        CASE
> > +            WHEN ((s.param2 & (2)::bigint) > 0) THEN 'end-of-recovery '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param2 & (4)::bigint) > 0) THEN 'immediate '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param2 & (8)::bigint) > 0) THEN 'force '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param2 & (16)::bigint) > 0) THEN 'flush-all '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param2 & (32)::bigint) > 0) THEN 'wait '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param2 & (128)::bigint) > 0) THEN 'wal '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param2 & (256)::bigint) > 0) THEN 'time '::text
> > +            ELSE ''::text
> > +        END) AS flags,
> > +    (((((((
> > +        CASE
> > +            WHEN ((s.param3 & (1)::bigint) > 0) THEN 'shutdown '::text
> > +            ELSE ''::text
> > +        END ||
> > +        CASE
> > +            WHEN ((s.param3 & (2)::bigint) > 0) THEN 'end-of-recovery '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param3 & (4)::bigint) > 0) THEN 'immediate '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param3 & (8)::bigint) > 0) THEN 'force '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param3 & (16)::bigint) > 0) THEN 'flush-all '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param3 & (32)::bigint) > 0) THEN 'wait '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param3 & (128)::bigint) > 0) THEN 'wal '::text
> > +            ELSE ''::text
> > +        END) ||
> > +        CASE
> > +            WHEN ((s.param3 & (256)::bigint) > 0) THEN 'time '::text
> > +            ELSE ''::text
> > +        END) AS next_flags,
> > +    ('0/0'::pg_lsn + (
> > +        CASE
> > +            WHEN (s.param4 < 0) THEN pow((2)::numeric, (64)::numeric)
> > +            ELSE (0)::numeric
> > +        END + (s.param4)::numeric)) AS start_lsn,
> > +    to_timestamp(((946684800)::double precision + ((s.param5)::double precision / (1000000)::double precision))) AS start_time,
> > +        CASE s.param6
> > +            WHEN 1 THEN 'initializing'::text
> > +            WHEN 2 THEN 'getting virtual transaction IDs'::text
> > +            WHEN 3 THEN 'checkpointing replication slots'::text
> > +            WHEN 4 THEN 'checkpointing logical replication snapshot files'::text
> > +            WHEN 5 THEN 'checkpointing logical rewrite mapping files'::text
> > +            WHEN 6 THEN 'checkpointing replication origin'::text
> > +            WHEN 7 THEN 'checkpointing commit log pages'::text
> > +            WHEN 8 THEN 'checkpointing commit time stamp pages'::text
> > +            WHEN 9 THEN 'checkpointing subtransaction pages'::text
> > +            WHEN 10 THEN 'checkpointing multixact pages'::text
> > +            WHEN 11 THEN 'checkpointing predicate lock pages'::text
> > +            WHEN 12 THEN 'checkpointing buffers'::text
> > +            WHEN 13 THEN 'processing file sync requests'::text
> > +            WHEN 14 THEN 'performing two phase checkpoint'::text
> > +            WHEN 15 THEN 'performing post checkpoint cleanup'::text
> > +            WHEN 16 THEN 'invalidating replication slots'::text
> > +            WHEN 17 THEN 'recycling old WAL files'::text
> > +            WHEN 18 THEN 'truncating subtransactions'::text
> > +            WHEN 19 THEN 'finalizing'::text
> > +            ELSE NULL::text
> > +        END AS phase,
> > +    s.param7 AS buffers_total,
> > +    s.param8 AS buffers_processed,
> > +    s.param9 AS buffers_written,
> > +    s.param10 AS files_total,
> > +    s.param11 AS files_synced
> > +   FROM pg_stat_get_progress_info('CHECKPOINT'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20);
> >  pg_stat_progress_cluster| SELECT s.pid,
> >      s.datid,
> >      d.datname,
>
> postgres[1160866][1]=# SELECT sum(octet_length(ev_action)), SUM(pg_column_size(ev_action)) FROM pg_rewrite WHERE ev_class::regclass::text LIKE '%progress%';
> ┌───────┬───────┐
> │  sum  │  sum  │
> ├───────┼───────┤
> │ 97410 │ 19786 │
> └───────┴───────┘
> (1 row)
>
> and this view looks to be a good bit more complicated than the existing
> pg_stat_progress* views.
>
> Indeed:
> template1[1165473][1]=# SELECT ev_class::regclass, length(ev_action), pg_column_size(ev_action) FROM pg_rewrite WHERE ev_class::regclass::text LIKE '%progress%' ORDER BY length(ev_action) DESC;
> ┌───────────────────────────────┬────────┬────────────────┐
> │           ev_class            │ length │ pg_column_size │
> ├───────────────────────────────┼────────┼────────────────┤
> │ pg_stat_progress_checkpoint   │  43290 │           5409 │
> │ pg_stat_progress_create_index │  23293 │           4177 │
> │ pg_stat_progress_cluster      │  18390 │           3704 │
> │ pg_stat_progress_analyze      │  16121 │           3339 │
> │ pg_stat_progress_vacuum       │  16076 │           3392 │
> │ pg_stat_progress_copy         │  15124 │           3080 │
> │ pg_stat_progress_basebackup   │   8406 │           2094 │
> └───────────────────────────────┴────────┴────────────────┘
> (7 rows)
>
> pg_rewrite without pg_stat_progress_checkpoint: 745472, with: 753664
>
>
> pg_rewrite is the second biggest relation in an empty database already...
>
> template1[1164827][1]=# SELECT relname, pg_total_relation_size(oid) FROM pg_class WHERE relkind = 'r' ORDER BY 2 DESC LIMIT 5;
> ┌────────────────┬────────────────────────┐
> │    relname     │ pg_total_relation_size │
> ├────────────────┼────────────────────────┤
> │ pg_proc        │                1212416 │
> │ pg_rewrite     │                 745472 │
> │ pg_attribute   │                 704512 │
> │ pg_description │                 630784 │
> │ pg_collation   │                 409600 │
> └────────────────┴────────────────────────┘
> (5 rows)
>
> Greetings,
>
> Andres Freund

---

From: Nitin Jadhav <[email-masked]>
Date: Mon, 13 Jun 2022 19:26:39 +0530

> We could also play it safe and only update the counters every N loops

The idea looks good but based on the performance numbers shared above,
it is not affecting the performance. So we can use the current
approach as it gives more accurate progress.
---

> any need for next_flags?

"next_flags" is removed in the v6 patch. Added a "new_requests" field
to get to know whether the current checkpoint is being throttled or
not. Please share your views on this.
---

> Second, is the start LSN really necessary
> for monitoring purposes?

IMO, start LSN is necessary to debug if the checkpoint is taking longer.
---

> Not all the information in the first
> seen as it is not possible to use a session at this stage, no?

I understand that "shutdown" and "end-of-recovery" will never be seen
and I have removed it in the v6 patch.
---

> There

"immediate" is required to understand whether the current checkpoint
is throttled or not. I am not sure about other flags "flush-all",
"force" and "wait". I have just supported all the flags to match the
'checkpoint start' log message. Please share your views. If it is not
really required, I will remove it in the next patch.
---

> not intuitive IMO, and callers of this routine will likely parse

If I understand the above comment correctly, you are saying to
introduce a new field, say "reason" ( possible values are either wal
or time) and the "flags" field will continue to represent the other
flags like "immediate", etc. The idea looks good here. We can
introduce new field "reason" and "flags" field can be renamed to
"throttled" (true/false) if we decide to not support other flags
"flush-all", "force" and "wait".
---

> shorter without losing their meaning.

I will try to make it short in the next patch.
---

Please share your thoughts.

Thanks & Regards,
Nitin Jadhav

---

From: Andres Freund <[email-masked]>
Date: Wed, 6 Jul 2022 17:04:46 -0700

Hi,

> environment. Here are the results.

Those look like timed checkpoints - if the checkpoints are sleeping a
part of the time, you're not going to see any potential overhead.

To see whether this has an effect you'd have to make sure there's a
certain number of dirty buffers (e.g. by doing CREATE TABLE AS
some_query) and then do a manual checkpoint and time how long that
times.

Greetings,

Andres Freund

---

From: Nitin Jadhav <[email-masked]>
Date: Thu, 28 Jul 2022 15:08:38 +0530

> part of the time, you're not going to see any potential overhead.

Yes. The above data is collected from timed checkpoints.

create table t1(a int);
insert into t1 select * from generate_series(1,10000000);

I generated a lot of data by using the above queries which would in
turn trigger the checkpoint (wal).
---

> some_query) and then do a manual checkpoint and time how long that

For this case I have generated data by using below queries.

create table t1(a int);
insert into t1 select * from generate_series(1,8000000);

This does not trigger the checkpoint automatically. I have issued the
CHECKPOINT manually and measured the performance by considering an
average of 5 checkpoints. Here are the details.

With patch: 2.457 s
Without patch: 2.334 s

Thanks & Regards,
Nitin Jadhav

---

From: "Drouvot, Bertrand" <[email-masked]>
Date: Fri, 4 Nov 2022 09:25:52 +0100

Hi,

> Please share your thoughts.

v6 was not applying anymore, due to a change in 
doc/src/sgml/ref/checkpoint.sgml done by b9eb0ff09e (Rename 
pg_checkpointer predefined role to pg_checkpoint).

Please find attached a rebase in v7.

While working on this rebase, I also noticed that "pg_checkpointer" is 
still mentioned in some translation files:
"
$ git grep pg_checkpointer
src/backend/po/de.po:msgid "must be superuser or have privileges of 
pg_checkpointer to do CHECKPOINT"
src/backend/po/ja.po:msgid "must be superuser or have privileges of 
pg_checkpointer to do CHECKPOINT"
src/backend/po/ja.po:msgstr 
"CHECKPOINTを実行するにはスーパーユーザーであるか、またはpg_checkpointerの権限を持つ必要があります"
src/backend/po/sv.po:msgid "must be superuser or have privileges of 
pg_checkpointer to do CHECKPOINT"
"

I'm not familiar with how the translation files are handled (looks like 
they have their own set of commits, see 3c0bcdbc66 for example) but 
wanted to mention that "pg_checkpointer" is still mentioned (even if 
that may be expected as the last commit related to translation files 
(aka 3c0bcdbc66) is older than the one that renamed pg_checkpointer to 
pg_checkpoint (aka b9eb0ff09e)).

That said, back to this patch: I did not look closely but noticed that 
the buffers_total reported by pg_stat_progress_checkpoint:

postgres=# select type,flags,start_lsn,phase,buffers_total,new_requests 
from pg_stat_progress_checkpoint;
     type    |         flags         | start_lsn  |         phase 
  | buffers_total | new_requests
------------+-----------------------+------------+-----------------------+---------------+--------------
  checkpoint | immediate force wait  | 1/E6C523A8 | checkpointing 
buffers |       1024275 | false
(1 row)

is a little bit different from what is logged once completed:

2022-11-04 08:18:50.806 UTC [3488442] LOG:  checkpoint complete: wrote 
1024278 buffers (97.7%);

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

commit 5198f5010be0febd019b1888817e2d78b8e42b21
Author: bdrouvot <[email-masked]>
Date:   Thu Nov 3 12:59:10 2022 +0000

    v7-0001-pg_stat_progress_checkpoint-view.patch

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index e5d622d514..ceb7d60ffa 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -414,6 +414,13 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
        See <xref linkend='copy-progress-reporting'/>.
       </entry>
      </row>
+
+     <row>
+      <entry><structname>pg_stat_progress_checkpoint</structname><indexterm><primary>pg_stat_progress_checkpoint</primary></indexterm></entry>
+      <entry>One row only, showing the progress of the checkpoint.
+       See <xref linkend='checkpoint-progress-reporting'/>.
+      </entry>
+     </row>
     </tbody>
    </tgroup>
   </table>
@@ -5784,7 +5791,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
    which support progress reporting are <command>ANALYZE</command>,
    <command>CLUSTER</command>,
    <command>CREATE INDEX</command>, <command>VACUUM</command>,
-   <command>COPY</command>,
+   <command>COPY</command>, <command>CHECKPOINT</command>
    and <xref linkend="protocol-replication-base-backup"/> (i.e., replication
    command that <xref linkend="app-pgbasebackup"/> issues to take
    a base backup).
@@ -7072,6 +7079,402 @@ FROM pg_stat_get_backend_idset() AS backendid;
   </table>
  </sect2>
 
+ <sect2 id="checkpoint-progress-reporting">
+  <title>Checkpoint Progress Reporting</title>
+
+  <indexterm>
+   <primary>pg_stat_progress_checkpoint</primary>
+  </indexterm>
+
+  <para>
+   Whenever the checkpoint operation is running, the
+   <structname>pg_stat_progress_checkpoint</structname> view will contain a
+   single row indicating the progress of the checkpoint. The tables below
+   describe the information that will be reported and provide information about
+   how to interpret it.
+  </para>
+
+  <table id="pg-stat-progress-checkpoint-view" xreflabel="pg_stat_progress_checkpoint">
+   <title><structname>pg_stat_progress_checkpoint</structname> View</title>
+   <tgroup cols="1">
+    <thead>
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       Column Type
+      </para>
+      <para>
+       Description
+      </para></entry>
+     </row>
+    </thead>
+
+    <tbody>
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>pid</structfield> <type>integer</type>
+      </para>
+      <para>
+       Process ID of the checkpointer process.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>type</structfield> <type>text</type>
+      </para>
+      <para>
+       Type of the checkpoint. See <xref linkend="checkpoint-types"/>.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>flags</structfield> <type>text</type>
+      </para>
+      <para>
+       Flags of the checkpoint. See <xref linkend="checkpoint-flags"/>.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>start_lsn</structfield> <type>text</type>
+      </para>
+      <para>
+       The checkpoint start location.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>start_time</structfield> <type>timestamp with time zone</type>
+      </para>
+      <para>
+       Start time of the checkpoint.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>phase</structfield> <type>text</type>
+      </para>
+      <para>
+       Current processing phase. See <xref linkend="checkpoint-phases"/>.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>buffers_total</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Total number of buffers to be written. This is estimated and reported
+       as of the beginning of buffer write operation.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>buffers_processed</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of buffers processed. This counter increases when the targeted
+       buffer is processed. This number will eventually become equal to
+       <literal>buffers_total</literal> when the checkpoint is
+       complete.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>buffers_written</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of buffers written. This counter only advances when the targeted
+       buffers is written. Note that some of the buffers are processed but may
+       not required to be written. So this count will always be  less than or
+       equal to  <literal>buffers_total</literal>.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>files_total</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Total number of files to be synced. This is estimated and reported as of
+       the beginning of sync operation.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>files_synced</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of files synced. This counter advances when the targeted file is
+       synced. This number will eventually become equal to
+       <literal>files_total</literal>  when the checkpoint is complete.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>new_requests</structfield> <type>text</type>
+      </para>
+      <para>
+       True if any of the backend is requested for a checkpoint while the
+       current checkpoint is in progress, False otherwise.
+      </para></entry>
+     </row>
+    </tbody>
+   </tgroup>
+  </table>
+
+  <table id="checkpoint-types">
+   <title>Checkpoint Types</title>
+   <tgroup cols="2">
+    <colspec colname="col1" colwidth="1*"/>
+    <colspec colname="col2" colwidth="2*"/>
+    <thead>
+     <row>
+      <entry>Types</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry><literal>checkpoint</literal></entry>
+      <entry>
+       The current operation is checkpoint.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>restartpoint</literal></entry>
+      <entry>
+       The current operation is restartpoint.
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+  </table>
+
+  <table id="checkpoint-flags">
+   <title>Checkpoint Flags</title>
+   <tgroup cols="2">
+    <colspec colname="col1" colwidth="1*"/>
+    <colspec colname="col2" colwidth="2*"/>
+    <thead>
+     <row>
+      <entry>Flags</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry><literal>shutdown</literal></entry>
+      <entry>
+       The checkpoint is for shutdown.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>end-of-recovery</literal></entry>
+      <entry>
+       The checkpoint is for end-of-recovery.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>immediate</literal></entry>
+      <entry>
+       The checkpoint happens without delays.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>force</literal></entry>
+      <entry>
+       The checkpoint is started because some operation (for which the
+       checkpoint is necessary) forced a checkpoint.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>flush all</literal></entry>
+      <entry>
+       The checkpoint flushes all pages, including those belonging to unlogged
+       tables.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>wait</literal></entry>
+      <entry>
+       The operations which requested the checkpoint waits for completion
+       before returning.
+      </entry>
+     </row>
+      <row>
+      <entry><literal>requested</literal></entry>
+      <entry>
+       The checkpoint request has been made.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>wal</literal></entry>
+      <entry>
+       The checkpoint is started because <literal>max_wal_size</literal> is
+       reached.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>time</literal></entry>
+      <entry>
+       The checkpoint is started because <literal>checkpoint_timeout</literal>
+       expired.
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+  </table>
+
+  <table id="checkpoint-phases">
+   <title>Checkpoint Phases</title>
+   <tgroup cols="2">
+    <colspec colname="col1" colwidth="1*"/>
+    <colspec colname="col2" colwidth="2*"/>
+    <thead>
+     <row>
+      <entry>Phase</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry><literal>initializing</literal></entry>
+      <entry>
+       The checkpointer process is preparing to begin the checkpoint operation.
+       This phase is expected to be very brief.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>getting virtual transaction IDs</literal></entry>
+      <entry>
+       The checkpointer process is getting the virtual transaction IDs that
+       are delaying the checkpoint.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing replication slots</literal></entry>
+      <entry>
+       The checkpointer process is currently flushing all the replication slots
+       to disk.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing logical replication snapshot files</literal></entry>
+      <entry>
+       The checkpointer process is currently removing all the serialized
+       snapshot files that are not required anymore.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing logical rewrite mapping files</literal></entry>
+      <entry>
+       The checkpointer process is currently removing unwanted or flushing
+       required logical rewrite mapping files.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing replication origin</literal></entry>
+      <entry>
+       The checkpointer process is currently performing a checkpoint of each
+       replication origin's progress with respect to the replayed remote LSN.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing commit log pages</literal></entry>
+      <entry>
+       The checkpointer process is currently writing commit log pages to disk.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing commit time stamp pages</literal></entry>
+      <entry>
+       The checkpointer process is currently writing commit time stamp pages to
+       disk.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing subtransaction pages</literal></entry>
+      <entry>
+       The checkpointer process is currently writing subtransaction pages to disk.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing multixact pages</literal></entry>
+      <entry>
+       The checkpointer process is currently writing multixact pages to disk.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing predicate lock pages</literal></entry>
+      <entry>
+       The checkpointer process is currently writing predicate lock pages to disk.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>checkpointing buffers</literal></entry>
+      <entry>
+       The checkpointer process is currently writing buffers to disk.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>processing file sync requests</literal></entry>
+      <entry>
+       The checkpointer process is currently processing file sync requests.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>performing two phase checkpoint</literal></entry>
+      <entry>
+       The checkpointer process is currently performing two phase checkpoint.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>performing post checkpoint cleanup</literal></entry>
+      <entry>
+       The checkpointer process is currently performing post checkpoint cleanup.
+       It removes any lingering files that can be safely removed.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>invalidating replication slots</literal></entry>
+      <entry>
+       The checkpointer process is currently invalidating replication slots.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>recycling old WAL files</literal></entry>
+      <entry>
+       The checkpointer process is currently recycling old WAL files.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>truncating subtransactions</literal></entry>
+      <entry>
+       The checkpointer process is currently removing the subtransaction
+       segments.
+      </entry>
+     </row>
+     <row>
+      <entry><literal>finalizing</literal></entry>
+      <entry>
+       The checkpointer process is finalizing the checkpoint operation.
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+  </table>
+
+ </sect2>
+
  </sect1>
 
  <sect1 id="dynamic-trace">
diff --git a/doc/src/sgml/ref/checkpoint.sgml b/doc/src/sgml/ref/checkpoint.sgml
index 28a1d717b8..906883336d 100644
--- a/doc/src/sgml/ref/checkpoint.sgml
+++ b/doc/src/sgml/ref/checkpoint.sgml
@@ -55,6 +55,14 @@ CHECKPOINT
    Only superusers or users with the privileges of
    the <link linkend="predefined-roles-table"><literal>pg_checkpoint</literal></link>
    role can call <command>CHECKPOINT</command>.
+
+  <para>
+    The checkpointer process running the checkpoint will report its progress
+    in the <structname>pg_stat_progress_checkpoint</structname> view except for
+    the shutdown and end-of-recovery cases. See
+    <xref linkend="checkpoint-progress-reporting"/> for details.
+  </para>
+
   </para>
  </refsect1>
 
diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml
index 6a38b53744..733a8d2837 100644
--- a/doc/src/sgml/wal.sgml
+++ b/doc/src/sgml/wal.sgml
@@ -531,7 +531,11 @@
    adjust the <xref linkend="guc-archive-timeout"/> parameter rather than the
    checkpoint parameters.)
    It is also possible to force a checkpoint by using the SQL
-   command <command>CHECKPOINT</command>.
+   command <command>CHECKPOINT</command>. The checkpointer process running the
+   checkpoint will report its progress in the
+   <structname>pg_stat_progress_checkpoint</structname> view except for the
+   shutdown and end-of-recovery cases. See
+   <xref linkend="checkpoint-progress-reporting"/> for details.
   </para>
 
   <para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index be54c23187..f2cebe0973 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -67,6 +67,7 @@
 #include "catalog/catversion.h"
 #include "catalog/pg_control.h"
 #include "catalog/pg_database.h"
+#include "commands/progress.h"
 #include "common/controldata_utils.h"
 #include "common/file_utils.h"
 #include "executor/instrument.h"
@@ -698,6 +699,8 @@ static void WALInsertLockAcquireExclusive(void);
 static void WALInsertLockRelease(void);
 static void WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt);
 
+static void checkpoint_progress_start(int flags, int type);
+
 /*
  * Insert an XLOG record represented by an already-constructed chain of data
  * chunks.  This is a low-level routine; to construct the WAL record header
@@ -6622,6 +6625,9 @@ CreateCheckPoint(int flags)
 	XLogCtl->RedoRecPtr = checkPoint.redo;
 	SpinLockRelease(&XLogCtl->info_lck);
 
+	/* Prepare to report progress of the checkpoint. */
+	checkpoint_progress_start(flags, PROGRESS_CHECKPOINT_TYPE_CHECKPOINT);
+
 	/*
 	 * If enabled, log checkpoint start.  We postpone this until now so as not
 	 * to log anything if we decided to skip the checkpoint.
@@ -6704,6 +6710,8 @@ CreateCheckPoint(int flags)
 	 * clog and we will correctly flush the update below.  So we cannot miss
 	 * any xacts we need to wait for.
 	 */
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_GET_VIRTUAL_TRANSACTION_IDS);
 	vxids = GetVirtualXIDsDelayingChkpt(&nvxids, DELAY_CHKPT_START);
 	if (nvxids > 0)
 	{
@@ -6819,6 +6827,8 @@ CreateCheckPoint(int flags)
 	/*
 	 * Let smgr do post-checkpoint cleanup (eg, deleting old files).
 	 */
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_POST_CHECKPOINT_CLEANUP);
 	SyncPostCheckpoint();
 
 	/*
@@ -6834,6 +6844,9 @@ CreateCheckPoint(int flags)
 	 */
 	XLByteToSeg(RedoRecPtr, _logSegNo, wal_segment_size);
 	KeepLogSeg(recptr, &_logSegNo);
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_INVALIDATE_REPLI_SLOTS);
+
 	if (InvalidateObsoleteReplicationSlots(_logSegNo))
 	{
 		/*
@@ -6844,6 +6857,8 @@ CreateCheckPoint(int flags)
 		KeepLogSeg(recptr, &_logSegNo);
 	}
 	_logSegNo--;
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_RECYCLE_OLD_XLOG);
 	RemoveOldXlogFiles(_logSegNo, RedoRecPtr, recptr,
 					   checkPoint.ThisTimeLineID);
 
@@ -6862,11 +6877,21 @@ CreateCheckPoint(int flags)
 	 * StartupSUBTRANS hasn't been called yet.
 	 */
 	if (!RecoveryInProgress())
+	{
+		pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+									 PROGRESS_CHECKPOINT_PHASE_TRUNCATE_SUBTRANS);
 		TruncateSUBTRANS(GetOldestTransactionIdConsideredRunning());
+	}
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_FINALIZE);
 
 	/* Real work is done; log and update stats. */
 	LogCheckpointEnd(false);
 
+	/* Stop reporting progress of the checkpoint. */
+	pgstat_progress_end_command();
+
 	/* Reset the process title */
 	update_checkpoint_display(flags, false, true);
 
@@ -7023,29 +7048,63 @@ static void
 CheckPointGuts(XLogRecPtr checkPointRedo, int flags)
 {
 	CheckPointRelationMap();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_REPLI_SLOTS);
 	CheckPointReplicationSlots();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_SNAPSHOTS);
 	CheckPointSnapBuild();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_LOGICAL_REWRITE_MAPPINGS);
 	CheckPointLogicalRewriteHeap();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_REPLI_ORIGIN);
 	CheckPointReplicationOrigin();
 
 	/* Write out all dirty data in SLRUs and the main buffer pool */
 	TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags);
 	CheckpointStats.ckpt_write_t = GetCurrentTimestamp();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_CLOG_PAGES);
 	CheckPointCLOG();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_COMMITTS_PAGES);
 	CheckPointCommitTs();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_SUBTRANS_PAGES);
 	CheckPointSUBTRANS();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_MULTIXACT_PAGES);
 	CheckPointMultiXact();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_PREDICATE_LOCK_PAGES);
 	CheckPointPredicate();
+
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_BUFFERS);
 	CheckPointBuffers(flags);
 
 	/* Perform all queued up fsyncs */
 	TRACE_POSTGRESQL_BUFFER_CHECKPOINT_SYNC_START();
 	CheckpointStats.ckpt_sync_t = GetCurrentTimestamp();
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_SYNC_FILES);
 	ProcessSyncRequests();
 	CheckpointStats.ckpt_sync_end_t = GetCurrentTimestamp();
 	TRACE_POSTGRESQL_BUFFER_CHECKPOINT_DONE();
 
 	/* We deliberately delay 2PC checkpointing as long as possible */
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_TWO_PHASE);
 	CheckPointTwoPhase(checkPointRedo);
 }
 
@@ -7195,6 +7254,9 @@ CreateRestartPoint(int flags)
 	MemSet(&CheckpointStats, 0, sizeof(CheckpointStats));
 	CheckpointStats.ckpt_start_t = GetCurrentTimestamp();
 
+	/* Prepare to report progress of the restartpoint. */
+	checkpoint_progress_start(flags, PROGRESS_CHECKPOINT_TYPE_RESTARTPOINT);
+
 	if (log_checkpoints)
 		LogCheckpointStart(flags, true);
 
@@ -7278,6 +7340,9 @@ CreateRestartPoint(int flags)
 	replayPtr = GetXLogReplayRecPtr(&replayTLI);
 	endptr = (receivePtr < replayPtr) ? replayPtr : receivePtr;
 	KeepLogSeg(endptr, &_logSegNo);
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_INVALIDATE_REPLI_SLOTS);
+
 	if (InvalidateObsoleteReplicationSlots(_logSegNo))
 	{
 		/*
@@ -7304,6 +7369,8 @@ CreateRestartPoint(int flags)
 	if (!RecoveryInProgress())
 		replayTLI = XLogCtl->InsertTimeLineID;
 
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_RECYCLE_OLD_XLOG);
 	RemoveOldXlogFiles(_logSegNo, RedoRecPtr, endptr, replayTLI);
 
 	/*
@@ -7320,11 +7387,20 @@ CreateRestartPoint(int flags)
 	 * this because StartupSUBTRANS hasn't been called yet.
 	 */
 	if (EnableHotStandby)
+	{
+		pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+									 PROGRESS_CHECKPOINT_PHASE_TRUNCATE_SUBTRANS);
 		TruncateSUBTRANS(GetOldestTransactionIdConsideredRunning());
+	}
 
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_PHASE,
+								 PROGRESS_CHECKPOINT_PHASE_FINALIZE);
 	/* Real work is done; log and update stats. */
 	LogCheckpointEnd(true);
 
+	/* Stop reporting progress of the restartpoint. */
+	pgstat_progress_end_command();
+
 	/* Reset the process title */
 	update_checkpoint_display(flags, true, true);
 
@@ -8958,3 +9034,29 @@ SetWalWriterSleeping(bool sleeping)
 	XLogCtl->WalWriterSleeping = sleeping;
 	SpinLockRelease(&XLogCtl->info_lck);
 }
+
+/*
+ * Start reporting progress of the checkpoint.
+ */
+static void
+checkpoint_progress_start(int flags, int type)
+{
+	const int	index[] = {
+		PROGRESS_CHECKPOINT_TYPE,
+		PROGRESS_CHECKPOINT_FLAGS,
+		PROGRESS_CHECKPOINT_LSN,
+		PROGRESS_CHECKPOINT_START_TIMESTAMP,
+		PROGRESS_CHECKPOINT_PHASE
+		};
+	int64		val[5];
+
+	pgstat_progress_start_command(PROGRESS_COMMAND_CHECKPOINT, InvalidOid);
+
+	val[0] = type;
+	val[1] = flags;
+	val[2] = RedoRecPtr;
+	val[3] = CheckpointStats.ckpt_start_t;
+	val[4] = PROGRESS_CHECKPOINT_PHASE_INIT;
+
+	pgstat_progress_update_multi_param(5, index, val);
+}
\ No newline at end of file
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 2d8104b090..384ca35833 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1267,6 +1267,57 @@ CREATE VIEW pg_stat_progress_copy AS
     FROM pg_stat_get_progress_info('COPY') AS S
         LEFT JOIN pg_database D ON S.datid = D.oid;
 
+CREATE VIEW pg_stat_progress_checkpoint AS
+    SELECT
+        S.pid AS pid,
+        CASE S.param1 WHEN 1 THEN 'checkpoint'
+                      WHEN 2 THEN 'restartpoint'
+                      END AS type,
+        ( CASE WHEN (S.param2 & 4) > 0 THEN 'immediate ' ELSE '' END ||
+          CASE WHEN (S.param2 & 8) > 0 THEN 'force ' ELSE '' END ||
+          CASE WHEN (S.param2 & 16) > 0 THEN 'flush-all ' ELSE '' END ||
+          CASE WHEN (S.param2 & 32) > 0 THEN 'wait ' ELSE '' END ||
+          CASE WHEN (S.param2 & 128) > 0 THEN 'wal ' ELSE '' END ||
+          CASE WHEN (S.param2 & 256) > 0 THEN 'time ' ELSE '' END
+        ) AS flags,
+        ( '0/0'::pg_lsn +
+          ((CASE
+                WHEN S.param3 < 0 THEN pow(2::numeric, 64::numeric)::numeric
+                ELSE 0::numeric
+            END) +
+           S.param3::numeric)
+        ) AS start_lsn,
+        to_timestamp(946684800 + (S.param4::float8 / 1000000)) AS start_time,
+        CASE S.param5 WHEN 1 THEN 'initializing'
+                      WHEN 2 THEN 'getting virtual transaction IDs'
+                      WHEN 3 THEN 'checkpointing replication slots'
+                      WHEN 4 THEN 'checkpointing logical replication snapshot files'
+                      WHEN 5 THEN 'checkpointing logical rewrite mapping files'
+                      WHEN 6 THEN 'checkpointing replication origin'
+                      WHEN 7 THEN 'checkpointing commit log pages'
+                      WHEN 8 THEN 'checkpointing commit time stamp pages'
+                      WHEN 9 THEN 'checkpointing subtransaction pages'
+                      WHEN 10 THEN 'checkpointing multixact pages'
+                      WHEN 11 THEN 'checkpointing predicate lock pages'
+                      WHEN 12 THEN 'checkpointing buffers'
+                      WHEN 13 THEN 'processing file sync requests'
+                      WHEN 14 THEN 'performing two phase checkpoint'
+                      WHEN 15 THEN 'performing post checkpoint cleanup'
+                      WHEN 16 THEN 'invalidating replication slots'
+                      WHEN 17 THEN 'recycling old WAL files'
+                      WHEN 18 THEN 'truncating subtransactions'
+                      WHEN 19 THEN 'finalizing'
+                      END AS phase,
+        S.param6 AS buffers_total,
+        S.param7 AS buffers_processed,
+        S.param8 AS buffers_written,
+        S.param9 AS files_total,
+        S.param10 AS files_synced,
+        CASE S.param11 WHEN 0 THEN 'false'
+                       WHEN 1 THEN 'true'
+                       END AS new_requests
+    FROM pg_stat_get_progress_info('CHECKPOINT') AS S;
+
 CREATE VIEW pg_user_mappings AS
     SELECT
         U.oid       AS umid,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 5fc076fc14..21bf75b058 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -39,6 +39,7 @@
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogrecovery.h"
+#include "commands/progress.h"
 #include "libpq/pqsignal.h"
 #include "miscadmin.h"
 #include "pgstat.h"
@@ -163,7 +164,7 @@ static pg_time_t last_xlog_switch_time;
 static void HandleCheckpointerInterrupts(void);
 static void CheckArchiveTimeout(void);
 static bool IsCheckpointOnSchedule(double progress);
-static bool ImmediateCheckpointRequested(void);
+static bool ImmediateCheckpointRequested(int flags);
 static bool CompactCheckpointerRequestQueue(void);
 static void UpdateSharedMemoryConfig(void);
 
@@ -667,16 +668,24 @@ CheckArchiveTimeout(void)
  * there is one pending behind it.)
  */
 static bool
-ImmediateCheckpointRequested(void)
+ImmediateCheckpointRequested(int flags)
 {
 	volatile CheckpointerShmemStruct *cps = CheckpointerShmem;
 
+	if (cps->ckpt_flags & CHECKPOINT_REQUESTED)
+		pgstat_progress_update_param(PROGRESS_CHECKPOINT_NEW_REQUESTS, true);
+
 	/*
 	 * We don't need to acquire the ckpt_lck in this case because we're only
 	 * looking at a single flag bit.
 	 */
 	if (cps->ckpt_flags & CHECKPOINT_IMMEDIATE)
+	{
+		pgstat_progress_update_param(PROGRESS_CHECKPOINT_FLAGS,
+									 (flags | CHECKPOINT_IMMEDIATE));
 		return true;
+	}
+
 	return false;
 }
 
@@ -708,7 +717,7 @@ CheckpointWriteDelay(int flags, double progress)
 	 */
 	if (!(flags & CHECKPOINT_IMMEDIATE) &&
 		!ShutdownRequestPending &&
-		!ImmediateCheckpointRequested() &&
+		!ImmediateCheckpointRequested(flags) &&
 		IsCheckpointOnSchedule(progress))
 	{
 		if (ConfigReloadPending)
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 73d30bf619..6d69255667 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -39,6 +39,7 @@
 #include "catalog/catalog.h"
 #include "catalog/storage.h"
 #include "catalog/storage_xlog.h"
+#include "commands/progress.h"
 #include "executor/instrument.h"
 #include "lib/binaryheap.h"
 #include "miscadmin.h"
@@ -2026,6 +2027,8 @@ BufferSync(int flags)
 	WritebackContextInit(&wb_context, &checkpoint_flush_after);
 
 	TRACE_POSTGRESQL_BUFFER_SYNC_START(NBuffers, num_to_scan);
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_BUFFERS_TOTAL,
+								 num_to_scan);
 
 	/*
 	 * Sort buffers that need to be written to reduce the likelihood of random
@@ -2143,6 +2146,8 @@ BufferSync(int flags)
 		bufHdr = GetBufferDescriptor(buf_id);
 
 		num_processed++;
 
 		/*
 		 * We don't need to acquire the lock here, because we're only looking
@@ -2163,6 +2168,8 @@ BufferSync(int flags)
 				TRACE_POSTGRESQL_BUFFER_SYNC_WRITTEN(buf_id);
 				PendingCheckpointerStats.buf_written_checkpoints++;
 				num_written++;
+				pgstat_progress_update_param(PROGRESS_CHECKPOINT_BUFFERS_WRITTEN,
+											 num_written);
 			}
 		}
 
diff --git a/src/backend/storage/sync/sync.c b/src/backend/storage/sync/sync.c
index 9d6a9e9109..aa25215910 100644
--- a/src/backend/storage/sync/sync.c
+++ b/src/backend/storage/sync/sync.c
@@ -23,6 +23,7 @@
 #include "access/multixact.h"
 #include "access/xlog.h"
 #include "access/xlogutils.h"
+#include "commands/progress.h"
 #include "commands/tablespace.h"
 #include "miscadmin.h"
 #include "pgstat.h"
@@ -368,6 +369,9 @@ ProcessSyncRequests(void)
 	/* Now scan the hashtable for fsync requests to process */
 	absorb_counter = FSYNCS_PER_ABSORB;
 	hash_seq_init(&hstat, pendingOps);
+	pgstat_progress_update_param(PROGRESS_CHECKPOINT_FILES_TOTAL,
+								 hash_get_num_entries(pendingOps));
+
 	while ((entry = (PendingFsyncEntry *) hash_seq_search(&hstat)) != NULL)
 	{
 		int			failures;
@@ -431,6 +435,8 @@ ProcessSyncRequests(void)
 						longest = elapsed;
 					total_elapsed += elapsed;
 					processed++;
+					pgstat_progress_update_param(PROGRESS_CHECKPOINT_FILES_SYNCED,
+												 processed);
 
 					if (log_checkpoints)
 						elog(DEBUG1, "checkpoint sync: number=%d file=%s time=%.3f ms",
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 96bffc0f2a..8281d961bf 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -497,6 +497,8 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
 		cmdtype = PROGRESS_COMMAND_BASEBACKUP;
 	else if (pg_strcasecmp(cmd, "COPY") == 0)
 		cmdtype = PROGRESS_COMMAND_COPY;
+	 else if (pg_strcasecmp(cmd, "CHECKPOINT") == 0)
+		cmdtype = PROGRESS_COMMAND_CHECKPOINT;
 	else
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index a28938caf4..33a64d2f0b 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -151,4 +151,42 @@
 #define PROGRESS_COPY_TYPE_PIPE 3
 #define PROGRESS_COPY_TYPE_CALLBACK 4
 
+/* Progress parameters for checkpoint */
+#define PROGRESS_CHECKPOINT_TYPE                    0
+#define PROGRESS_CHECKPOINT_FLAGS                   1
+#define PROGRESS_CHECKPOINT_LSN                     2
+#define PROGRESS_CHECKPOINT_START_TIMESTAMP         3
+#define PROGRESS_CHECKPOINT_PHASE                   4
+#define PROGRESS_CHECKPOINT_BUFFERS_TOTAL           5
+#define PROGRESS_CHECKPOINT_BUFFERS_PROCESSED       6
+#define PROGRESS_CHECKPOINT_BUFFERS_WRITTEN         7
+#define PROGRESS_CHECKPOINT_FILES_TOTAL             8
+#define PROGRESS_CHECKPOINT_FILES_SYNCED            9
+#define PROGRESS_CHECKPOINT_NEW_REQUESTS            10
+
+/* Types of checkpoint (as advertised via PROGRESS_CHECKPOINT_TYPE) */
+#define PROGRESS_CHECKPOINT_TYPE_CHECKPOINT         1
+#define PROGRESS_CHECKPOINT_TYPE_RESTARTPOINT       2
+
+/* Phases of checkpoint (as advertised via PROGRESS_CHECKPOINT_PHASE) */
+#define PROGRESS_CHECKPOINT_PHASE_INIT                          1
+#define PROGRESS_CHECKPOINT_PHASE_GET_VIRTUAL_TRANSACTION_IDS   2
+#define PROGRESS_CHECKPOINT_PHASE_REPLI_SLOTS                   3
+#define PROGRESS_CHECKPOINT_PHASE_SNAPSHOTS                     4
+#define PROGRESS_CHECKPOINT_PHASE_LOGICAL_REWRITE_MAPPINGS      5
+#define PROGRESS_CHECKPOINT_PHASE_REPLI_ORIGIN                  6
+#define PROGRESS_CHECKPOINT_PHASE_CLOG_PAGES                    7
+#define PROGRESS_CHECKPOINT_PHASE_COMMITTS_PAGES                8
+#define PROGRESS_CHECKPOINT_PHASE_SUBTRANS_PAGES                9
+#define PROGRESS_CHECKPOINT_PHASE_MULTIXACT_PAGES               10
+#define PROGRESS_CHECKPOINT_PHASE_PREDICATE_LOCK_PAGES          11
+#define PROGRESS_CHECKPOINT_PHASE_BUFFERS                       12
+#define PROGRESS_CHECKPOINT_PHASE_SYNC_FILES                    13
+#define PROGRESS_CHECKPOINT_PHASE_TWO_PHASE                     14
+#define PROGRESS_CHECKPOINT_PHASE_POST_CHECKPOINT_CLEANUP       15
+#define PROGRESS_CHECKPOINT_PHASE_INVALIDATE_REPLI_SLOTS        16
+#define PROGRESS_CHECKPOINT_PHASE_RECYCLE_OLD_XLOG              17
+#define PROGRESS_CHECKPOINT_PHASE_TRUNCATE_SUBTRANS             18
+#define PROGRESS_CHECKPOINT_PHASE_FINALIZE                      19
+
 #endif
diff --git a/src/include/utils/backend_progress.h b/src/include/utils/backend_progress.h
index 47bf8029b0..02d51fb948 100644
--- a/src/include/utils/backend_progress.h
+++ b/src/include/utils/backend_progress.h
@@ -27,7 +27,8 @@ typedef enum ProgressCommandType
 	PROGRESS_COMMAND_CLUSTER,
 	PROGRESS_COMMAND_CREATE_INDEX,
 	PROGRESS_COMMAND_BASEBACKUP,
-	PROGRESS_COMMAND_COPY
+	PROGRESS_COMMAND_COPY,
+	PROGRESS_COMMAND_CHECKPOINT
 } ProgressCommandType;
 
 #define PGSTAT_NUM_PROGRESS_PARAM	20
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 624d0e5aae..eab68d7f14 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1913,6 +1913,76 @@ pg_stat_progress_basebackup| SELECT s.pid,
     s.param4 AS tablespaces_total,
     s.param5 AS tablespaces_streamed
    FROM pg_stat_get_progress_info('BASEBACKUP'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20);
+pg_stat_progress_checkpoint| SELECT s.pid,
+        CASE s.param1
+            WHEN 1 THEN 'checkpoint'::text
+            WHEN 2 THEN 'restartpoint'::text
+            ELSE NULL::text
+        END AS type,
+    (((((
+        CASE
+            WHEN ((s.param2 & (4)::bigint) > 0) THEN 'immediate '::text
+            ELSE ''::text
+        END ||
+        CASE
+            WHEN ((s.param2 & (8)::bigint) > 0) THEN 'force '::text
+            ELSE ''::text
+        END) ||
+        CASE
+            WHEN ((s.param2 & (16)::bigint) > 0) THEN 'flush-all '::text
+            ELSE ''::text
+        END) ||
+        CASE
+            WHEN ((s.param2 & (32)::bigint) > 0) THEN 'wait '::text
+            ELSE ''::text
+        END) ||
+        CASE
+            WHEN ((s.param2 & (128)::bigint) > 0) THEN 'wal '::text
+            ELSE ''::text
+        END) ||
+        CASE
+            WHEN ((s.param2 & (256)::bigint) > 0) THEN 'time '::text
+            ELSE ''::text
+        END) AS flags,
+    ('0/0'::pg_lsn + (
+        CASE
+            WHEN (s.param3 < 0) THEN pow((2)::numeric, (64)::numeric)
+            ELSE (0)::numeric
+        END + (s.param3)::numeric)) AS start_lsn,
+    to_timestamp(((946684800)::double precision + ((s.param4)::double precision / (1000000)::double precision))) AS start_time,
+        CASE s.param5
+            WHEN 1 THEN 'initializing'::text
+            WHEN 2 THEN 'getting virtual transaction IDs'::text
+            WHEN 3 THEN 'checkpointing replication slots'::text
+            WHEN 4 THEN 'checkpointing logical replication snapshot files'::text
+            WHEN 5 THEN 'checkpointing logical rewrite mapping files'::text
+            WHEN 6 THEN 'checkpointing replication origin'::text
+            WHEN 7 THEN 'checkpointing commit log pages'::text
+            WHEN 8 THEN 'checkpointing commit time stamp pages'::text
+            WHEN 9 THEN 'checkpointing subtransaction pages'::text
+            WHEN 10 THEN 'checkpointing multixact pages'::text
+            WHEN 11 THEN 'checkpointing predicate lock pages'::text
+            WHEN 12 THEN 'checkpointing buffers'::text
+            WHEN 13 THEN 'processing file sync requests'::text
+            WHEN 14 THEN 'performing two phase checkpoint'::text
+            WHEN 15 THEN 'performing post checkpoint cleanup'::text
+            WHEN 16 THEN 'invalidating replication slots'::text
+            WHEN 17 THEN 'recycling old WAL files'::text
+            WHEN 18 THEN 'truncating subtransactions'::text
+            WHEN 19 THEN 'finalizing'::text
+            ELSE NULL::text
+        END AS phase,
+    s.param6 AS buffers_total,
+    s.param7 AS buffers_processed,
+    s.param8 AS buffers_written,
+    s.param9 AS files_total,
+    s.param10 AS files_synced,
+        CASE s.param11
+            WHEN 0 THEN 'false'::text
+            WHEN 1 THEN 'true'::text
+            ELSE NULL::text
+        END AS new_requests
+   FROM pg_stat_get_progress_info('CHECKPOINT'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20);
 pg_stat_progress_cluster| SELECT s.pid,
     s.datid,
     d.datname,

---

From: Nitin Jadhav <[email-masked]>
Date: Tue, 15 Nov 2022 17:11:52 +0530

> still mentioned in some translation files:

Thanks for rebasing the patch and sharing the information.
---

>  | buffers_total | new_requests

This is because the count shown in the checkpoint complete message
includes the additional increment done during SlruInternalWritePage().
We are not sure of this increment until it really happens. Hence it
was not considered in the patch. To make it compatible with the
checkpoint complete message, we should increment all three here,
buffers_total, buffers_processed and buffers_written. So the total
number of buffers calculated earlier may not always be the same. If
this looks good, I will update this in the next patch.

Thanks & Regards,
Nitin Jadhav

---

From: Robert Haas <[email-masked]>
Date: Tue, 15 Nov 2022 15:04:59 -0500

> Please find attached a rebase in v7.

I don't think it's a good thing that this patch is using the
progress-reporting machinery. The point of that machinery is that we
want any backend to be able to report progress for any command it
happens to be running, and we don't know which command that will be at
any given point in time, or how many backends will be running any
given command at once. So we need some generic set of counters that
can be repurposed for whatever any particular backend happens to be
doing right at the moment.

But none of that applies to the checkpointer. Any information about
the checkpointer that we want to expose can just be advertised in a
dedicated chunk of shared memory, perhaps even by simply adding it to
CheckpointerShmemStruct. Then you can give the fields whatever names,
types, and sizes you like, and you don't have to do all of this stuff
with mapping down to integers and back. The only real disadvantage
that I can see is then you have to think a bit harder about what the
concurrency model is here, and maybe you end up reimplementing
something similar to what the progress-reporting stuff does for you,
and *maybe* that is a sufficient reason to do it this way.

But I'm doubtful. This feels like a square-peg-round-hole situation.

--
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Andres Freund <[email-masked]>
Date: Tue, 15 Nov 2022 12:18:11 -0800

Hi,

>  	CheckPointTwoPhase(checkPointRedo);

This is quite the code bloat. Can we make this less duplicative?

> +                WHEN S.param3 < 0 THEN pow(2::numeric, 64::numeric)::numeric

I don't think we should embed this much complexity in the view
defintions. It's hard to read, bloats the catalog, we can't fix them once
released.  This stuff seems like it should be in a helper function.

I don't have any iea what that pow stuff is supposed to be doing.

> +        to_timestamp(946684800 + (S.param4::float8 / 1000000)) AS start_time,

I don't think this is a reasonable path - embedding way too much low-level
details about the timestamp format in the view definition. Why do we need to
do this?

Greetings,

Andres Freund

---

From: Bharath Rupireddy <[email-masked]>
Date: Wed, 16 Nov 2022 16:01:55 +0530

> doing right at the moment.

Hm.

> and *maybe* that is a sufficient reason to do it this way.

-1 for CheckpointerShmemStruct as it is being used for running
checkpoints and I don't think adding stats to it is a great idea.
Instead, extending PgStat_CheckpointerStats and using shared memory
stats for reporting progress/last checkpoint related stats is a good
idea IMO. I also think that a new pg_stat_checkpoint view is needed
because, right now, the PgStat_CheckpointerStats stats are exposed via
the pg_stat_bgwriter view, having a separate view for checkpoint stats
is good here. Also, removing CheckpointStatsData and moving all of
those members to PgStat_CheckpointerStats, of course, by being careful
about the amount of shared memory required, is also a good idea IMO.
Going forward, PgStat_CheckpointerStats and pg_stat_checkpoint view
can be a single point of location for all the checkpoint related
stats.

Thoughts?

In fact, I was recently having an off-list chat with Bertrand Drouvot
about the above idea.

--
Bharath Rupireddy

---

From: Andres Freund <[email-masked]>
Date: Wed, 16 Nov 2022 10:14:33 -0800

Hi,

> checkpoints and I don't think adding stats to it is a great idea.

Why?  Imo the data needed for progress reporting aren't really "stats". We'd
not accumulate counters over time, just for the current checkpoint.

I think it might even be useful for other parts of the system to know what the
checkpointer is doing, e.g. bgwriter or autovacuum could adapt the behaviour
if checkpointer can't keep up. Somehow it'd feel wrong to use the stats system
as the source of such adjustments - but perhaps my gut feeling on that isn't
right.

The best argument for combining progress reporting with accumulating stats is
that we could likely share some of the code. Having accumulated stats for all
the checkpoint phases would e.g. be quite valuable.

> idea IMO

There's certainly some potential for deduplicating state and to make stats
updated more frequently. But that doesn't necessarily mean that putting the
checkpoint progress into PgStat_CheckpointerStats is a good idea (nor the
opposite).

> I also think that a new pg_stat_checkpoint view is needed
> is good here.

I agree that we should do that, but largely independent of the architectural
question at hand.

Greetings,

Andres Freund

---

From: Robert Haas <[email-masked]>
Date: Wed, 16 Nov 2022 14:19:32 -0500

On Wed, Nov 16, 2022 at 5:32 AM Bharath Rupireddy
<[email-masked]> wrote:
> idea IMO.

I agree with Andres: progress reporting isn't really quite the same
thing as stats, and either place seems like it could be reasonable. I
don't presently have an opinion on which is a better fit, but I don't
think the fact that CheckpointerShmemStruct is used for running
checkpoints rules anything out. Progress reporting is *also* about
running checkpoints. Any historical data you want to expose might not
be about running checkpoints, but, uh, so what? I don't really see
that as a strong argument against it fitting into this struct.

> the pg_stat_bgwriter view, having a separate view for checkpoint stats

Yep.

> Also, removing CheckpointStatsData and moving all of

I'm not sure that I completely follow this part, or that I agree with
it. I have never really understood why we drive background writer or
checkpointer statistics through the statistics collector. Here again,
for things like table statistics, there is no choice, because we could
have an unbounded number of tables and need to keep statistics about
all of them. The statistics collector can handle that by allocating
more memory as required. But there is only one background writer and
only one checkpointer, so that is not needed in those cases. Why not
just have them expose anything they want to expose through shared
memory directly?

If the statistics collector provides services that we care about, like
persisting data across restarts or making snapshots for transactional
behavior, then those might be reasons to go through it even for the
background writer or checkpointer. But if so, we should be explicit
about what the reasons are, both in the mailing list discussion and in
code comments. Otherwise I fear that we'll just end up doing something
in a more complicated way than is really necessary.

---

From: Andres Freund <[email-masked]>
Date: Wed, 16 Nov 2022 11:52:32 -0800

Hi,

On 2022-11-16 14:19:32 -0500, Robert Haas wrote:
> I have never really understood why we drive background writer or
> checkpointer statistics through the statistics collector.

To some degree it is required for durability - the stats system needs to know
how to write out those stats. But that wasn't ever a good reason to send
messages to the stats collector - it could just read the stats from shared
memory after all.

There's also integration with snapshots of the stats, resetting them, etc.

There's also the complexity that some of the stats e.g. for checkpointer
aren't about work the checkpointer did, but just have ended up there for
historical raisins. E.g. the number of fsyncs and writes done by backends.

See below:

> Here again, for things like table statistics, there is no choice, because we
> could have an unbounded number of tables and need to keep statistics about
> all of them. The statistics collector can handle that by allocating more
> memory as required. But there is only one background writer and only one
> checkpointer, so that is not needed in those cases. Why not just have them
> expose anything they want to expose through shared memory directly?

That's how it is in 15+. The memory for "fixed-numbered" or "global"
statistics are maintained by the stats system, but in plain shared memory,
allocated at server start. Not via the hash table.

Right now stats updates for the checkpointer use the "changecount" approach to
updates. For now that makes sense, because we update the stats only
occasionally (after a checkpoint or when writing in CheckpointWriteDelay()) -
a stats viewer seeing the checkpoint count go up, without yet seeing the
corresponding buffers written would be misleading.

I don't think we'd want every buffer write or whatnot go through the
changecount mechanism, on some non-x86 platforms that could be noticable. But
if we didn't stage the stats updates locally I think we could make most of the
stats changes without that overhead.  For updates that just increment a single
counter there's simply no benefit in the changecount mechanism afaict.

I didn't want to do that change during the initial shared memory stats work,
it already was bigger than I could handle...

It's not quite clear to me what the best path forward is for
buf_written_backend / buf_fsync_backend, which currently are reported via the
checkpointer stats. I think the best path might be to stop counting them via
the CheckpointerShmem->num_backend_writes etc and just populate the fields in
the view (for backward compat) via the proposed [1] pg_stat_io patch.  Doing
that accounting with CheckpointerCommLock held exclusively isn't free.

> in a more complicated way than is really necessary.

I tried to provide at least some of that in the comments at the start of
pgstat.c in 15+. There's very likely more that should be added, but I think
it's a decent start.

Greetings,

Andres Freund

[1] https://www.postgresql.org/message-id/CAOtHd0ApHna7_p6mvHoO%2BgLZdxjaQPRemg3_o0a4ytCPijLytQ%40mail.gmail.com

---

From: Bharath Rupireddy <[email-masked]>
Date: Thu, 17 Nov 2022 19:01:45 +0530

> I agree that we should do that, but largely independent of the architectural

Thanks. I quickly prepared a patch introducing pg_stat_checkpointer
view and posted it here -
https://www.postgresql.org/message-id/CALj2ACVxX2ii%3D66RypXRweZe2EsBRiPMj0aHfRfHUeXJcC7kHg%40mail.gmail.com.

---

From: Robert Haas <[email-masked]>
Date: Thu, 17 Nov 2022 09:03:32 -0500

> counter there's simply no benefit in the changecount mechanism afaict.

You might be right, but I'm not sure whether it's worth stressing
about. The progress reporting mechanism uses the st_changecount
mechanism, too, and as far as I know nobody's complained about that
having too much overhead. Maybe they have, though, and I've just
missed it.

---

From: Andres Freund <[email-masked]>
Date: Thu, 17 Nov 2022 08:24:31 -0800

Hi,

> having too much overhead. Maybe they have, though, and I've just

I've seen it in profiles, although not as the major contributor. Most places
do a reasonable amount of work between calls though.

As an experiment, I added a progress report to BufferSync()'s first loop
(i.e. where it checks all buffers). On a 128GB shared_buffers cluster that
increases the time for a do-nothing checkpoint from ~235ms to ~280ms. If I
remove the changecount stuff and use a single write + write barrier, it ends
up as 250ms. Inlining brings it down a bit further, to 247ms.

Obviously this is a very extreme case - we only do very little work between
the progress report calls. But it does seem to show that the overhead is not
entirely neglegible.

I think pgstat_progress_start_command() needs the changecount stuff, as does
pgstat_progress_update_multi_param(). But for anything updating a single
parameter at a time it really doesn't do anything useful on a platform that
doesn't tear 64bit writes (so it could be #ifdef
PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY).

Out of further curiosity I wanted to test the impact when the loop doesn't
even do a LockBufHdr() and added an unlocked pre-check. 109ms without
progress. 138ms with. 114ms with the simplified
pgstat_progress_update_param(). 108ms after inlining the simplified
pgstat_progress_update_param().

Greetings,

Andres Freund

---

From: Tom Lane <[email-masked]>
Date: Thu, 17 Nov 2022 12:18:00 -0500

> PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY).

Seems safe to restrict it to that case.

			regards, tom lane

---

From: Robert Haas <[email-masked]>
Date: Thu, 17 Nov 2022 12:21:03 -0500

> up as 250ms. Inlining brings it down a bit further, to 247ms.

OK, I'd say that's pretty good evidence that we can't totally
disregard the issue.

---

From: Andres Freund <[email-masked]>
Date: Wed, 7 Dec 2022 11:03:49 -0800

Hi,

> Please find attached a rebase in v7.

cfbot complains that the docs don't build:
https://cirrus-ci.com/task/6694349031866368?logs=docs_build#L296

[03:24:27.317] ref/checkpoint.sgml:66: element para: validity error : Element para is not declared in para list of possible children

I've marked the patch as waitin-on-author for now.

There's been a bunch of architectural feedback too, but tbh, I don't know if
we came to any conclusion on that front...

Greetings,

Andres Freund