Horton

Thread

Commits

  1. Fix inconsistent reporting of checkpointer stats.

  2. Defer flushing of SLRU files.

  1. Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2022-12-14T07:31:49Z

    Hi,

While working on checkpoint related stuff, I have encountered that
there is some inconsistency while reporting checkpointer stats. When a
checkpoint gets completed, a checkpoint complete message gets logged.
This message has a lot of information including the buffers written
(CheckpointStats.ckpt_bufs_written). This variable gets incremented in
2 cases. First is in BufferSync() and the second is in
SlruInternalWritePage(). On the other hand the checkpointer stats
exposed using pg_stat_bgwriter contains a lot of information including
buffers written (PendingCheckpointerStats.buf_written_checkpoints).
This variable gets incremented in only one place and that is in
BufferSync(). So there is inconsistent behaviour between these two
data. Please refer to the sample output below.

postgres=# select * from pg_stat_bgwriter;
 checkpoints_timed | checkpoints_req | checkpoint_write_time |
checkpoint_sync_time | buffers_checkpoint | buffers_clean |
maxwritten_clean | buffers_backend | buffers_backend_fsync |
buffers_alloc |          stats_reset
-------------------+-----------------+-----------------------+----------------------+--------------------+---------------+------------------+-----------------+-----------------------+---------------+-------------------------------
                 0 |               1 |                    75 |
         176 |               4702 |             0 |                0 |
           4656 |                     0 |          5023 | 2022-12-14
07:01:01.494672+00

2022-12-14 07:03:18.052 UTC [6087] LOG:  checkpoint starting:
immediate force wait
2022-12-14 07:03:18.370 UTC [6087] LOG:  checkpoint complete: wrote
4705 buffers (28.7%); 0 WAL file(s) added, 0 removed, 4 recycled;
write=0.075 s, sync=0.176 s, total=0.318 s; sync files=34,
longest=0.159 s, average=0.006 s; distance=66180 kB, estimate=66180
kB; lsn=0/5565E38, redo lsn=0/5565E00


In order to fix this, the
PendingCheckpointerStats.buf_written_checkpoints should be incremented
in SlruInternalWritePage() similar to
CheckpointStats.ckpt_bufs_written. I have attached the patch for the
same. Please share your thoughts.


Thanks & Regards,
Nitin Jadhav

  2. Re: Inconsistency in reporting checkpointer stats

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-12-14T11:24:53Z

    On Wed, Dec 14, 2022 at 1:02 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
>
> Hi,
>
> While working on checkpoint related stuff, I have encountered that
> there is some inconsistency while reporting checkpointer stats. When a
> checkpoint gets completed, a checkpoint complete message gets logged.
> This message has a lot of information including the buffers written
> (CheckpointStats.ckpt_bufs_written). This variable gets incremented in
> 2 cases. First is in BufferSync() and the second is in
> SlruInternalWritePage(). On the other hand the checkpointer stats
> exposed using pg_stat_bgwriter contains a lot of information including
> buffers written (PendingCheckpointerStats.buf_written_checkpoints).
> This variable gets incremented in only one place and that is in
> BufferSync(). So there is inconsistent behaviour between these two
> data. Please refer to the sample output below.
>
> In order to fix this, the
> PendingCheckpointerStats.buf_written_checkpoints should be incremented
> in SlruInternalWritePage() similar to
> CheckpointStats.ckpt_bufs_written. I have attached the patch for the
> same. Please share your thoughts.

Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count
buffer writes in SlruInternalWritePage(). However, does it need to be
done immediately there? The stats will not be visible to the users
until the next pgstat_report_checkpointer(). Incrementing
buf_written_checkpoints in BufferSync() makes sense as the
pgstat_report_checkpointer() gets called in there via
CheckpointWriteDelay() and it becomes visible to the user immediately.
Have you checked if pgstat_report_checkpointer() gets called while the
checkpoint calls SlruInternalWritePage()? If not, then you can just
assign ckpt_bufs_written to buf_written_checkpoints in
LogCheckpointEnd() like its other friends
checkpoint_write_time and checkpoint_sync_time.

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

  3. Re: Inconsistency in reporting checkpointer stats

    Michael Paquier <michael@paquier.xyz> — 2022-12-14T21:46:04Z

    On Wed, Dec 14, 2022 at 04:54:53PM +0530, Bharath Rupireddy wrote:
> Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count
> buffer writes in SlruInternalWritePage(). However, does it need to be
> done immediately there? The stats will not be visible to the users
> until the next pgstat_report_checkpointer(). Incrementing
> buf_written_checkpoints in BufferSync() makes sense as the
> pgstat_report_checkpointer() gets called in there via
> CheckpointWriteDelay() and it becomes visible to the user immediately.
> Have you checked if pgstat_report_checkpointer() gets called while the
> checkpoint calls SlruInternalWritePage()? If not, then you can just
> assign ckpt_bufs_written to buf_written_checkpoints in
> LogCheckpointEnd() like its other friends
> checkpoint_write_time and checkpoint_sync_time.

    /* If part of a checkpoint, count this as a buffer written. */
    if (fdata)
        CheckpointStats.ckpt_bufs_written++;
+       PendingCheckpointerStats.buf_written_checkpoints++;
Also, the proposed patch would touch PendingCheckpointerStats even
when there is no fdata, aka outside the context of a checkpoint or
shutdown sequence..
--
Michael

  4. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2022-12-15T10:12:00Z

    > Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count
> buffer writes in SlruInternalWritePage(). However, does it need to be
> done immediately there? The stats will not be visible to the users
> until the next pgstat_report_checkpointer(). Incrementing
> buf_written_checkpoints in BufferSync() makes sense as the
> pgstat_report_checkpointer() gets called in there via
> CheckpointWriteDelay() and it becomes visible to the user immediately.
> Have you checked if pgstat_report_checkpointer() gets called while the
> checkpoint calls SlruInternalWritePage()? If not, then you can just
> assign ckpt_bufs_written to buf_written_checkpoints in
> LogCheckpointEnd() like its other friends
> checkpoint_write_time and checkpoint_sync_time.

In case of an immediate checkpoint, the CheckpointWriteDelay() never
gets called until the checkpoint is completed. So no issues in this
case. CheckpointWriteDelay() comes into picture in case of non
immediate checkpoints (i.e. checkpoint timeout is reached or max wal
size is reached). If we remove the increment in BufferSync() and
SlruInternalWritePage() and then just assign ckpt_bufs_written to
buf_written_checkpoints in LogCheckpointEnd() then the update will be
available after the end of each checkpoint which is not better than
the existing behaviour (without patch). If we keep the increment in
BufferSync() then we have to calculate the remaining buffer
incremented in SlruInternalWritePage() and then increment
buf_written_checkpoints with this number in LogCheckpointEnd(). This
just makes it complicated and again the buffer incremented in
SlruInternalWritePage() will get updated at the end of the checkpoint.
In the case of checkpoint_write_time and checkpoint_sync_time, it
makes sense because this information is based on the entire checkpoint
operation and it should be done at the end. So I feel the patch
handles it in a better way even though the
pgstat_report_checkpointer() does not get called immediately but it
will be called during the next increment in BufferSync() which is
before the end of the checkpoint. Please share if you have any other
ideas.

On Wed, Dec 14, 2022 at 4:55 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, Dec 14, 2022 at 1:02 PM Nitin Jadhav
> <nitinjadhavpostgres@gmail.com> wrote:
> >
> > Hi,
> >
> > While working on checkpoint related stuff, I have encountered that
> > there is some inconsistency while reporting checkpointer stats. When a
> > checkpoint gets completed, a checkpoint complete message gets logged.
> > This message has a lot of information including the buffers written
> > (CheckpointStats.ckpt_bufs_written). This variable gets incremented in
> > 2 cases. First is in BufferSync() and the second is in
> > SlruInternalWritePage(). On the other hand the checkpointer stats
> > exposed using pg_stat_bgwriter contains a lot of information including
> > buffers written (PendingCheckpointerStats.buf_written_checkpoints).
> > This variable gets incremented in only one place and that is in
> > BufferSync(). So there is inconsistent behaviour between these two
> > data. Please refer to the sample output below.
> >
> > In order to fix this, the
> > PendingCheckpointerStats.buf_written_checkpoints should be incremented
> > in SlruInternalWritePage() similar to
> > CheckpointStats.ckpt_bufs_written. I have attached the patch for the
> > same. Please share your thoughts.
>
> Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count
> buffer writes in SlruInternalWritePage(). However, does it need to be
> done immediately there? The stats will not be visible to the users
> until the next pgstat_report_checkpointer(). Incrementing
> buf_written_checkpoints in BufferSync() makes sense as the
> pgstat_report_checkpointer() gets called in there via
> CheckpointWriteDelay() and it becomes visible to the user immediately.
> Have you checked if pgstat_report_checkpointer() gets called while the
> checkpoint calls SlruInternalWritePage()? If not, then you can just
> assign ckpt_bufs_written to buf_written_checkpoints in
> LogCheckpointEnd() like its other friends
> checkpoint_write_time and checkpoint_sync_time.
>
> --
> Bharath Rupireddy
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com

  5. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2022-12-15T10:13:35Z

    >  /* If part of a checkpoint, count this as a buffer written. */
>    if (fdata)
>        CheckpointStats.ckpt_bufs_written++;
> +       PendingCheckpointerStats.buf_written_checkpoints++;
> Also, the proposed patch would touch PendingCheckpointerStats even
> when there is no fdata, aka outside the context of a checkpoint or
> shutdown sequence.

Sorry. I missed adding braces. Fixed in the v2 patch attached.

Thanks & Regards,
Nitin Jadhav

On Thu, Dec 15, 2022 at 3:16 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Dec 14, 2022 at 04:54:53PM +0530, Bharath Rupireddy wrote:
> > Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count
> > buffer writes in SlruInternalWritePage(). However, does it need to be
> > done immediately there? The stats will not be visible to the users
> > until the next pgstat_report_checkpointer(). Incrementing
> > buf_written_checkpoints in BufferSync() makes sense as the
> > pgstat_report_checkpointer() gets called in there via
> > CheckpointWriteDelay() and it becomes visible to the user immediately.
> > Have you checked if pgstat_report_checkpointer() gets called while the
> > checkpoint calls SlruInternalWritePage()? If not, then you can just
> > assign ckpt_bufs_written to buf_written_checkpoints in
> > LogCheckpointEnd() like its other friends
> > checkpoint_write_time and checkpoint_sync_time.
>
>     /* If part of a checkpoint, count this as a buffer written. */
>     if (fdata)
>         CheckpointStats.ckpt_bufs_written++;
> +       PendingCheckpointerStats.buf_written_checkpoints++;
> Also, the proposed patch would touch PendingCheckpointerStats even
> when there is no fdata, aka outside the context of a checkpoint or
> shutdown sequence..
> --
> Michael

  6. Re: Inconsistency in reporting checkpointer stats

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-12-16T08:43:43Z

    At Wed, 14 Dec 2022 16:54:53 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in 
> Indeed PendingCheckpointerStats.buf_written_checkpoints needs to count
> buffer writes in SlruInternalWritePage(). However, does it need to be
> done immediately there? The stats will not be visible to the users
> until the next pgstat_report_checkpointer(). Incrementing
> buf_written_checkpoints in BufferSync() makes sense as the
> pgstat_report_checkpointer() gets called in there via
> CheckpointWriteDelay() and it becomes visible to the user immediately.
> Have you checked if pgstat_report_checkpointer() gets called while the
> checkpoint calls SlruInternalWritePage()? If not, then you can just
> assign ckpt_bufs_written to buf_written_checkpoints in
> LogCheckpointEnd() like its other friends
> checkpoint_write_time and checkpoint_sync_time.

If I'm getting Bharath correctly, it results in double counting of
BufferSync. If we want to keep the realtime-reporting nature of
BufferSync, BufferSync should give up to increment CheckPointerStats'
counter.  Such separation seems to be a kind of stupid and quite
bug-prone.

In the first place I don't like that we count the same things twice.
Couldn't we count the number only by any one of them?

If we remove CheckPointerStats.ckpt_bufs_written, CreateCheckPoint can
get the final number as the difference between the start-end values of
*the shared stats*. As long as a checkpoint runs on a single process,
trace info in BufferSync will work fine.  Assuming single process
checkpointing there must be no problem to do that. (Anyway the current
shared stats update for checkpointer is assuming single-process).

Otherwise, in exchange with giving up the realtime nature, we can
count the number only by CheckPointerStats.ckpt_bufs_written.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  7. Re: Inconsistency in reporting checkpointer stats

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-12-19T12:35:38Z

    On Fri, Dec 16, 2022 at 2:14 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> In the first place I don't like that we count the same things twice.
> Couldn't we count the number only by any one of them?
>
> If we remove CheckPointerStats.ckpt_bufs_written, CreateCheckPoint can
> get the final number as the difference between the start-end values of
> *the shared stats*. As long as a checkpoint runs on a single process,
> trace info in BufferSync will work fine.  Assuming single process
> checkpointing there must be no problem to do that. (Anyway the current
> shared stats update for checkpointer is assuming single-process).

What if someone resets checkpointer shared stats with
pg_stat_reset_shared()? In such a case, the checkpoint complete
message will not have the stats, no?

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

  8. Re: Inconsistency in reporting checkpointer stats

    Robert Haas <robertmhaas@gmail.com> — 2022-12-19T21:08:23Z

    On Wed, Dec 14, 2022 at 2:32 AM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
> In order to fix this, the
> PendingCheckpointerStats.buf_written_checkpoints should be incremented
> in SlruInternalWritePage() similar to
> CheckpointStats.ckpt_bufs_written. I have attached the patch for the
> same. Please share your thoughts.

Presumably we could make this consistent either by counting SLRU
writes in both places, or by counting them in neither place. This
proposal would count them in both places. But why is that the right
thing to do?

I'm somewhat inclined to think that we should use "buffers" to mean
regular data buffers, and if SLRU buffers also need to be counted, we
ought to make that a separate counter. Or just leave it out
altogether.

This is arguable, though, for sure....

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

  9. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2022-12-20T13:03:18Z

    > Presumably we could make this consistent either by counting SLRU
> writes in both places, or by counting them in neither place. This
> proposal would count them in both places. But why is that the right
> thing to do?
>
> I'm somewhat inclined to think that we should use "buffers" to mean
> regular data buffers, and if SLRU buffers also need to be counted, we
> ought to make that a separate counter. Or just leave it out
> altogether.
>
> This is arguable, though, for sure....

Thanks Robert for sharing your thoughts.
My first thought was to just remove counting SLRU buffers, then after
some more analysis, I found that the checkpointer is responsible for
including both regular data buffers and SLRU buffers. Please refer to
dee663f7843902535a15ae366cede8b4089f1144 commit for more information.
The part of the commit message is included here [1] for quick
reference. Hence I concluded to keep the information and added an
increment to count SLRU buffers. I am not in favour of making this as
a separate counter as this can be treated as little low level
information and it just adds up in the stats. Please share your
thoughts.

[1]:
Hoist ProcessSyncRequests() up into CheckPointGuts() to make it clearer
that it applies to all the SLRU mini-buffer-pools as well as the main
buffer pool.  Rearrange things so that data collected in CheckpointStats
includes SLRU activity.

Thanks & Regards,
Nitin Jadhav

On Tue, Dec 20, 2022 at 2:38 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Dec 14, 2022 at 2:32 AM Nitin Jadhav
> <nitinjadhavpostgres@gmail.com> wrote:
> > In order to fix this, the
> > PendingCheckpointerStats.buf_written_checkpoints should be incremented
> > in SlruInternalWritePage() similar to
> > CheckpointStats.ckpt_bufs_written. I have attached the patch for the
> > same. Please share your thoughts.
>
> Presumably we could make this consistent either by counting SLRU
> writes in both places, or by counting them in neither place. This
> proposal would count them in both places. But why is that the right
> thing to do?
>
> I'm somewhat inclined to think that we should use "buffers" to mean
> regular data buffers, and if SLRU buffers also need to be counted, we
> ought to make that a separate counter. Or just leave it out
> altogether.
>
> This is arguable, though, for sure....
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com

  10. Re: Inconsistency in reporting checkpointer stats

    Robert Haas <robertmhaas@gmail.com> — 2022-12-20T13:18:36Z

    On Tue, Dec 20, 2022 at 8:03 AM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
> Thanks Robert for sharing your thoughts.
> My first thought was to just remove counting SLRU buffers, then after
> some more analysis, I found that the checkpointer is responsible for
> including both regular data buffers and SLRU buffers.

I know that, but what the checkpointer handles and what ought to be
included in the stats are two separate questions.

I think that the SLRU information is potentially useful, but mixing it
with the information about regular buffers just seems confusing.

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

  11. Re: Inconsistency in reporting checkpointer stats

    Andres Freund <andres@anarazel.de> — 2022-12-20T17:38:20Z

    On 2022-12-20 08:18:36 -0500, Robert Haas wrote:
> I think that the SLRU information is potentially useful, but mixing it
> with the information about regular buffers just seems confusing.

+1

At least for now, it'd be different if/when we manage to move SLRUs to
the main buffer pool.

- Andres

  12. Re: Inconsistency in reporting checkpointer stats

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-12-21T11:32:39Z

    On Tue, Dec 20, 2022 at 11:08 PM Andres Freund <andres@anarazel.de> wrote:
>
> On 2022-12-20 08:18:36 -0500, Robert Haas wrote:
> > I think that the SLRU information is potentially useful, but mixing it
> > with the information about regular buffers just seems confusing.
>
> +1
>
> At least for now, it'd be different if/when we manage to move SLRUs to
> the main buffer pool.

+1 to not count SLRU writes in ckpt_bufs_written. If needed we can
have new fields CheckpointStats.ckpt_slru_bufs_written and
PendingCheckpointerStats.slru_buf_written_checkpoint.

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

  13. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2022-12-21T11:44:12Z

    Thanks Robert and Andres for sharing your thoughts.

I have modified the code accordingly and attached the new version of
patches. patch 0001 fixes the inconsistency in checkpointer stats and
patch 0002 separates main buffer and SLRU buffer count from checkpoint
complete log message. In 0001, I added a new column to
pg_stat_bgwriter view and named it as slru_buffers_checkpoint and kept
the existing column buffers_checkpoint as-is. Should I rename this to
something like main_buffers_checkpoint? Thoughts?

Please refer to sample checkpoint complete log message[1]. I am not
quite satisfied with the percentage of buffers written information
logged there. The percentage is calculated based on NBuffers in both
the cases but I am just worried that are we passing wrong information
to the user while user may
think that the percentage of buffers is based on the total number of
buffers available and the percentage of SLRU buffers is based on the
total number of SLRU buffers available.

Kindly review and share your comments.

[1]:
2022-12-21 10:52:25.931 UTC [63530] LOG:  checkpoint complete: wrote
4670 buffers (28.5%), wrote 3 slru buffers (0.0%); 0 WAL file(s)
added, 0 removed, 4 recycled; write=0.045 s, sync=0.161 s, total=0.244
s; sync files=25, longest=0.146 s, average=0.007 s; distance=66130 kB,
estimate=66130 kB; lsn=0/5557C78, redo lsn=0/5557C40

Thanks & Regards,
Nitin Jadhav

On Tue, Dec 20, 2022 at 11:08 PM Andres Freund <andres@anarazel.de> wrote:
>
> On 2022-12-20 08:18:36 -0500, Robert Haas wrote:
> > I think that the SLRU information is potentially useful, but mixing it
> > with the information about regular buffers just seems confusing.
>
> +1
>
> At least for now, it'd be different if/when we manage to move SLRUs to
> the main buffer pool.
>
> - Andres

  14. Re: Inconsistency in reporting checkpointer stats

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-12-22T02:31:25Z

    At Mon, 19 Dec 2022 18:05:38 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in 
> On Fri, Dec 16, 2022 at 2:14 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > In the first place I don't like that we count the same things twice.
> > Couldn't we count the number only by any one of them?
> >
> > If we remove CheckPointerStats.ckpt_bufs_written, CreateCheckPoint can
> > get the final number as the difference between the start-end values of
> > *the shared stats*. As long as a checkpoint runs on a single process,
> > trace info in BufferSync will work fine.  Assuming single process
> > checkpointing there must be no problem to do that. (Anyway the current
> > shared stats update for checkpointer is assuming single-process).
> 
> What if someone resets checkpointer shared stats with
> pg_stat_reset_shared()? In such a case, the checkpoint complete
> message will not have the stats, no?

I don't know. I don't believe the stats system doesn't follow such a
strict resetting policy.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  15. Re: Inconsistency in reporting checkpointer stats

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2022-12-22T02:54:28Z

    At Wed, 21 Dec 2022 17:14:12 +0530, Nitin Jadhav <nitinjadhavpostgres@gmail.com> wrote in 
> [1]:
> 2022-12-21 10:52:25.931 UTC [63530] LOG:  checkpoint complete: wrote
> 4670 buffers (28.5%), wrote 3 slru buffers (0.0%); 0 WAL file(s)
> added, 0 removed, 4 recycled; write=0.045 s, sync=0.161 s, total=0.244
> s; sync files=25, longest=0.146 s, average=0.007 s; distance=66130 kB,
> estimate=66130 kB; lsn=0/5557C78, redo lsn=0/5557C40
> 
> Thanks & Regards,
> Nitin Jadhav
> 
> On Tue, Dec 20, 2022 at 11:08 PM Andres Freund <andres@anarazel.de> wrote:
> >
> > On 2022-12-20 08:18:36 -0500, Robert Haas wrote:
> > > I think that the SLRU information is potentially useful, but mixing it
> > > with the information about regular buffers just seems confusing.
> >
> > +1
> >
> > At least for now, it'd be different if/when we manage to move SLRUs to
> > the main buffer pool.

It sounds reasonable to exclude SRLU write from buffer writes. But I'm
not sure its useful to count SLRU writes separately since it is under
the noise level of buffer writes in reglular cases and the value
doesn't lead to tuning. However I'm not strongly opposed to adding it
either.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  16. Re: Inconsistency in reporting checkpointer stats

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2023-01-27T05:15:08Z

    On Wed, Dec 21, 2022 at 5:15 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
>
> I have modified the code accordingly and attached the new version of
> patches. patch 0001 fixes the inconsistency in checkpointer stats and
> patch 0002 separates main buffer and SLRU buffer count from checkpoint
> complete log message.

IMO, there's no need for 2 separate patches for these changes.

+                (errmsg("restartpoint complete: wrote %d buffers (%.1f%%), "
+                        "wrote %d slru buffers (%.1f%%); %d WAL
file(s) added, "
+                        "%d removed, %d recycled; write=%ld.%03d s, "
+                        "sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, "
+                        "longest=%ld.%03d s, average=%ld.%03d s;
distance=%d kB, "
+                        "estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X",
Hm, restartpoint /checkpoint complete message is already too long to
read and adding slru buffers to it make it further longer. Note that
we don't need to add every checkpoint stat to the log message but to
pg_stat_bgwriter. Isn't it enough to show SLRU buffers information in
pg_stat_bgwriter alone?

Can't one look at pg_stat_slru's blks_written
(pgstat_count_slru_page_written()) to really track the SLRUs written?
Or is it that one may want to track SLRUs during a checkpoint
separately? Is there a real use-case/customer reported issue driving
this change?

After looking at pg_stat_slru.blks_written, I think the best way is to
just leave things as-is and let CheckpointStats count slru buffers too
unless there's really a reported issue that says
pg_stat_slru.blks_written doesn't serve the purpose.

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

  17. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2023-01-27T14:25:04Z

    > IMO, there's no need for 2 separate patches for these changes.

I will make it a single patch while sharing the next patch.


> +                (errmsg("restartpoint complete: wrote %d buffers (%.1f%%), "
> +                        "wrote %d slru buffers (%.1f%%); %d WAL
> file(s) added, "
> +                        "%d removed, %d recycled; write=%ld.%03d s, "
> +                        "sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, "
> +                        "longest=%ld.%03d s, average=%ld.%03d s;
> distance=%d kB, "
> +                        "estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X",
> Hm, restartpoint /checkpoint complete message is already too long to
> read and adding slru buffers to it make it further longer. Note that
> we don't need to add every checkpoint stat to the log message but to
> pg_stat_bgwriter. Isn't it enough to show SLRU buffers information in
> pg_stat_bgwriter alone?

I understand that the log message is too long already but I feel it is
ok since it logs only one time per checkpoint and as discussed
upthread, SLRU information is potentially useful.


> Can't one look at pg_stat_slru's blks_written
> (pgstat_count_slru_page_written()) to really track the SLRUs written?
> Or is it that one may want to track SLRUs during a checkpoint
> separately? Is there a real use-case/customer reported issue driving
> this change?
>
> After looking at pg_stat_slru.blks_written, I think the best way is to
> just leave things as-is and let CheckpointStats count slru buffers too
> unless there's really a reported issue that says
> pg_stat_slru.blks_written doesn't serve the purpose.

The v1 patch corresponds to what you are suggesting. But the question
is not about tracking slru buffers, it is about separating this
information from main buffers count during checkpoint. I think there
is enough discussion done upthread to exclude slru buffers from
CheckpointStats.ckpt_bufs_written. Please share if you still strongly
feel that a separate counter is not required.


Thanks & Regards,
Nitin Jadhav

On Fri, Jan 27, 2023 at 10:45 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, Dec 21, 2022 at 5:15 PM Nitin Jadhav
> <nitinjadhavpostgres@gmail.com> wrote:
> >
> > I have modified the code accordingly and attached the new version of
> > patches. patch 0001 fixes the inconsistency in checkpointer stats and
> > patch 0002 separates main buffer and SLRU buffer count from checkpoint
> > complete log message.
>
> IMO, there's no need for 2 separate patches for these changes.
>
> +                (errmsg("restartpoint complete: wrote %d buffers (%.1f%%), "
> +                        "wrote %d slru buffers (%.1f%%); %d WAL
> file(s) added, "
> +                        "%d removed, %d recycled; write=%ld.%03d s, "
> +                        "sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, "
> +                        "longest=%ld.%03d s, average=%ld.%03d s;
> distance=%d kB, "
> +                        "estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X",
> Hm, restartpoint /checkpoint complete message is already too long to
> read and adding slru buffers to it make it further longer. Note that
> we don't need to add every checkpoint stat to the log message but to
> pg_stat_bgwriter. Isn't it enough to show SLRU buffers information in
> pg_stat_bgwriter alone?
>
> Can't one look at pg_stat_slru's blks_written
> (pgstat_count_slru_page_written()) to really track the SLRUs written?
> Or is it that one may want to track SLRUs during a checkpoint
> separately? Is there a real use-case/customer reported issue driving
> this change?
>
> After looking at pg_stat_slru.blks_written, I think the best way is to
> just leave things as-is and let CheckpointStats count slru buffers too
> unless there's really a reported issue that says
> pg_stat_slru.blks_written doesn't serve the purpose.
>
> --
> Bharath Rupireddy
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com

  18. Re: Inconsistency in reporting checkpointer stats

    Andres Freund <andres@anarazel.de> — 2023-02-14T00:38:01Z

    On 2022-12-21 17:14:12 +0530, Nitin Jadhav wrote:
> Kindly review and share your comments.

This doesn't pass the tests, because the regression tests weren't adjusted:

https://api.cirrus-ci.com/v1/artifact/task/5937624817336320/testrun/build/testrun/regress/regress/regression.diffs

  19. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2023-02-19T09:57:31Z

    > This doesn't pass the tests, because the regression tests weren't adjusted:
> https://api.cirrus-ci.com/v1/artifact/task/5937624817336320/testrun/build/testrun/regress/regress/regression.diffs

Thanks for sharing this. I have fixed this in the patch attached.


>> IMO, there's no need for 2 separate patches for these changes.
> I will make it a single patch while sharing the next patch.

Clubbed both patches into one.

Thanks & Regards,
Nitin Jadhav

On Tue, Feb 14, 2023 at 6:08 AM Andres Freund <andres@anarazel.de> wrote:
>
>
> On 2022-12-21 17:14:12 +0530, Nitin Jadhav wrote:
> > Kindly review and share your comments.
>
> This doesn't pass the tests, because the regression tests weren't adjusted:
>
> https://api.cirrus-ci.com/v1/artifact/task/5937624817336320/testrun/build/testrun/regress/regress/regression.diffs

  20. Re: Inconsistency in reporting checkpointer stats

    Gregory Stark (as CFM) <stark.cfm@gmail.com> — 2023-03-20T20:01:02Z

    On Sun, 19 Feb 2023 at 04:58, Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
>
> > This doesn't pass the tests, because the regression tests weren't adjusted:
> > https://api.cirrus-ci.com/v1/artifact/task/5937624817336320/testrun/build/testrun/regress/regress/regression.diffs
>
> Thanks for sharing this. I have fixed this in the patch attached.

AFAICS this patch was marked Waiting on Author due to this feedback on
Feb 14 but the patch was updated Feb 19 and is still passing today.
I've marked it Needs Review.

I'm not sure if all of the feedback from Bharath Rupireddy and Kyotaro
Horiguchi was addressed.

If there's any specific questions remaining you need feedback on it
would be good to ask explicitly. Otherwise if you think it's ready you
could mark it Ready for Commit.




-- 
Gregory Stark
As Commitfest Manager

  21. Re: Inconsistency in reporting checkpointer stats

    vignesh C <vignesh21@gmail.com> — 2024-01-20T02:40:03Z

    On Sun, 19 Feb 2023 at 15:28, Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
>
> > This doesn't pass the tests, because the regression tests weren't adjusted:
> > https://api.cirrus-ci.com/v1/artifact/task/5937624817336320/testrun/build/testrun/regress/regress/regression.diffs
>
> Thanks for sharing this. I have fixed this in the patch attached.
>
>
> >> IMO, there's no need for 2 separate patches for these changes.
> > I will make it a single patch while sharing the next patch.
>
> Clubbed both patches into one.

The patch does not apply anymore, please post a rebased version of the patch :
1 out of 1 hunk FAILED -- saving rejects to file
src/backend/catalog/system_views.sql.rej
patching file src/backend/utils/activity/pgstat_checkpointer.c
Hunk #1 FAILED at 52.
1 out of 1 hunk FAILED -- saving rejects to file
src/backend/utils/activity/pgstat_checkpointer.c.rej
patching file src/backend/utils/adt/pgstatfuncs.c
Hunk #1 succeeded at 1217 with fuzz 1 (offset 24 lines).
patching file src/include/access/xlog.h
Hunk #1 succeeded at 165 (offset 3 lines).
patching file src/include/catalog/pg_proc.dat
Hunk #1 FAILED at 5680.
1 out of 1 hunk FAILED -- saving rejects to file
src/include/catalog/pg_proc.dat.rej

Regards,
Vignesh

  22. Re: Inconsistency in reporting checkpointer stats

    Michael Paquier <michael@paquier.xyz> — 2024-01-30T07:50:02Z

    On Sat, Jan 20, 2024 at 08:10:03AM +0530, vignesh C wrote:
> The patch does not apply anymore, please post a rebased version of the patch :

There is more to it.  Some of the columns of pg_stat_bgwriter have
been moved to a different view, aka pg_stat_checkpointer.  I have
marked the patch as returned with feedback for now, so feel free to
resubmit if you can get a new version of the patch.
--
Michael

  23. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2024-07-18T07:08:00Z

    I apologize for not being active on this thread. However, I have now
returned to the thread and confirmed that the inconsistency is still
present in the latest code. I believe it’s crucial to address this
issue, and I am currently submitting the v5 version of the patch. The
v4 version had addressed the feedback from Bharath, Kyotaro, Andres,
and Robert. The current version has been rebased to incorporate
Vignesh’s suggestions. In response to Michael’s comments, I’ve moved
the new ‘slru_written’ column from the ‘pg_stat_bgwriter’ view to the
‘pg_stat_checkpointer’ in the attached patch.

To summarize our discussions, we’ve reached a consensus to correct the
mismatch between the information on buffers written as displayed in
the ‘pg_stat_checkpointer’ view and the checkpointer log message.
We’ve also agreed to separate the SLRU buffers data from the buffers
written and present the SLRU buffers data in a distinct field.

I have created the new commitfest entry here
https://commitfest.postgresql.org/49/5130/.
Kindly share if any comments.

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft


On Tue, Jan 30, 2024 at 1:20 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sat, Jan 20, 2024 at 08:10:03AM +0530, vignesh C wrote:
> > The patch does not apply anymore, please post a rebased version of the patch :
>
> There is more to it.  Some of the columns of pg_stat_bgwriter have
> been moved to a different view, aka pg_stat_checkpointer.  I have
> marked the patch as returned with feedback for now, so feel free to
> resubmit if you can get a new version of the patch.
> --
> Michael

  24. Re: Inconsistency in reporting checkpointer stats

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-09-18T13:27:29Z

    
On 2024/07/18 16:08, Nitin Jadhav wrote:
> I apologize for not being active on this thread. However, I have now
> returned to the thread and confirmed that the inconsistency is still
> present in the latest code. I believe it’s crucial to address this
> issue, and I am currently submitting the v5 version of the patch. The
> v4 version had addressed the feedback from Bharath, Kyotaro, Andres,
> and Robert. The current version has been rebased to incorporate
> Vignesh’s suggestions. In response to Michael’s comments, I’ve moved
> the new ‘slru_written’ column from the ‘pg_stat_bgwriter’ view to the
> ‘pg_stat_checkpointer’ in the attached patch.
> 
> To summarize our discussions, we’ve reached a consensus to correct the
> mismatch between the information on buffers written as displayed in
> the ‘pg_stat_checkpointer’ view and the checkpointer log message.
> We’ve also agreed to separate the SLRU buffers data from the buffers
> written and present the SLRU buffers data in a distinct field.
> 
> I have created the new commitfest entry here
> https://commitfest.postgresql.org/49/5130/.
> Kindly share if any comments.

Thanks for updating the patch!

In pgstat_checkpointer.c, it looks like you missed adding
CHECKPOINTER_COMP(slru_written) in pgstat_checkpointer_snapshot_cb().

+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+        <structfield>slru_written</structfield> <type>bigint</type>
+      </para>
+      <para>
+        Number of SLRU buffers written during checkpoints and restartpoints
+      </para></entry>
+     </row>

This entry should be moved to the pg_stat_checkpointer documentation.

+						CheckpointStats.ckpt_slru_written,
+						(double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,

I don't think NBuffers represents the maximum number of SLRU buffers.
We might need to calculate this based on specific GUC settings,
like transaction_buffers.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  25. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2024-09-22T11:44:31Z

    Thanks for the review.

> In pgstat_checkpointer.c, it looks like you missed adding
> CHECKPOINTER_COMP(slru_written) in pgstat_checkpointer_snapshot_cb().

Fixed it.


> +     <row>
> +      <entry role="catalog_table_entry"><para role="column_definition">
> +        <structfield>slru_written</structfield> <type>bigint</type>
> +      </para>
> +      <para>
> +        Number of SLRU buffers written during checkpoints and restartpoints
> +      </para></entry>
> +     </row>
>
> This entry should be moved to the pg_stat_checkpointer documentation.

Fixed it.


> +                                               CheckpointStats.ckpt_slru_written,
> +                                               (double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
>
> I don't think NBuffers represents the maximum number of SLRU buffers.
> We might need to calculate this based on specific GUC settings,
> like transaction_buffers.

Great observation. Since the SLRU buffers written during a checkpoint
can include transaction_buffers, commit_timestamp_buffers,
subtransaction_buffers, multixact_member_buffers,
multixact_offset_buffers, and serializable_buffers, the total count of
SLRU buffers should be the sum of all these types. We might need to
introduce a global variable, such as total_slru_count, in the
globals.c file to hold this sum. The num_slots variable in the
SlruSharedData structure needs to be accessed from all types of SLRU
and stored in total_slru_count. This can then be used during logging
to calculate the percentage of SLRU buffers written. However, I’m
unsure if this effort is justified. While it makes sense for normal
buffers to display the percentage, the additional code required might
not provide significant value to users. Therefore, I have removed this
in the attached v6 patch. If it is really required, I am happy to make
the above changes and share the updated patch.

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft

On Wed, Sep 18, 2024 at 6:57 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2024/07/18 16:08, Nitin Jadhav wrote:
> > I apologize for not being active on this thread. However, I have now
> > returned to the thread and confirmed that the inconsistency is still
> > present in the latest code. I believe it’s crucial to address this
> > issue, and I am currently submitting the v5 version of the patch. The
> > v4 version had addressed the feedback from Bharath, Kyotaro, Andres,
> > and Robert. The current version has been rebased to incorporate
> > Vignesh’s suggestions. In response to Michael’s comments, I’ve moved
> > the new ‘slru_written’ column from the ‘pg_stat_bgwriter’ view to the
> > ‘pg_stat_checkpointer’ in the attached patch.
> >
> > To summarize our discussions, we’ve reached a consensus to correct the
> > mismatch between the information on buffers written as displayed in
> > the ‘pg_stat_checkpointer’ view and the checkpointer log message.
> > We’ve also agreed to separate the SLRU buffers data from the buffers
> > written and present the SLRU buffers data in a distinct field.
> >
> > I have created the new commitfest entry here
> > https://commitfest.postgresql.org/49/5130/.
> > Kindly share if any comments.
>
> Thanks for updating the patch!
>
> In pgstat_checkpointer.c, it looks like you missed adding
> CHECKPOINTER_COMP(slru_written) in pgstat_checkpointer_snapshot_cb().
>
> +     <row>
> +      <entry role="catalog_table_entry"><para role="column_definition">
> +        <structfield>slru_written</structfield> <type>bigint</type>
> +      </para>
> +      <para>
> +        Number of SLRU buffers written during checkpoints and restartpoints
> +      </para></entry>
> +     </row>
>
> This entry should be moved to the pg_stat_checkpointer documentation.
>
> +                                               CheckpointStats.ckpt_slru_written,
> +                                               (double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
>
> I don't think NBuffers represents the maximum number of SLRU buffers.
> We might need to calculate this based on specific GUC settings,
> like transaction_buffers.
>
> Regards,
>
> --
> Fujii Masao
> Advanced Computing Technology Center
> Research and Development Headquarters
> NTT DATA CORPORATION
>

  26. Re: Inconsistency in reporting checkpointer stats

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-09-30T18:33:30Z

    
On 2024/09/22 20:44, Nitin Jadhav wrote:
>> +                                               CheckpointStats.ckpt_slru_written,
>> +                                               (double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
>>
>> I don't think NBuffers represents the maximum number of SLRU buffers.
>> We might need to calculate this based on specific GUC settings,
>> like transaction_buffers.
> 
> Great observation. Since the SLRU buffers written during a checkpoint
> can include transaction_buffers, commit_timestamp_buffers,
> subtransaction_buffers, multixact_member_buffers,
> multixact_offset_buffers, and serializable_buffers, the total count of
> SLRU buffers should be the sum of all these types. We might need to
> introduce a global variable, such as total_slru_count, in the
> globals.c file to hold this sum. The num_slots variable in the
> SlruSharedData structure needs to be accessed from all types of SLRU
> and stored in total_slru_count. This can then be used during logging
> to calculate the percentage of SLRU buffers written. However, I’m
> unsure if this effort is justified. While it makes sense for normal
> buffers to display the percentage, the additional code required might
> not provide significant value to users. Therefore, I have removed this
> in the attached v6 patch.

+1

Thanks for updating the patch! It looks good to me.
Barring any objections, I will commit this patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  27. Re: Inconsistency in reporting checkpointer stats

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-10-01T15:58:08Z

    
On 2024/10/01 3:33, Fujii Masao wrote:
> 
> 
> On 2024/09/22 20:44, Nitin Jadhav wrote:
>>> +                                               CheckpointStats.ckpt_slru_written,
>>> +                                               (double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
>>>
>>> I don't think NBuffers represents the maximum number of SLRU buffers.
>>> We might need to calculate this based on specific GUC settings,
>>> like transaction_buffers.
>>
>> Great observation. Since the SLRU buffers written during a checkpoint
>> can include transaction_buffers, commit_timestamp_buffers,
>> subtransaction_buffers, multixact_member_buffers,
>> multixact_offset_buffers, and serializable_buffers, the total count of
>> SLRU buffers should be the sum of all these types. We might need to
>> introduce a global variable, such as total_slru_count, in the
>> globals.c file to hold this sum. The num_slots variable in the
>> SlruSharedData structure needs to be accessed from all types of SLRU
>> and stored in total_slru_count. This can then be used during logging
>> to calculate the percentage of SLRU buffers written. However, I’m
>> unsure if this effort is justified. While it makes sense for normal
>> buffers to display the percentage, the additional code required might
>> not provide significant value to users. Therefore, I have removed this
>> in the attached v6 patch.
> 
> +1
> 
> Thanks for updating the patch! It looks good to me.
> Barring any objections, I will commit this patch.

Before committing the patch, I revised it with the following changes:

- I added "shared" to the description of pg_stat_checkpointer.buffers_written
   to clarify that it tracks shared buffers.
- In the checkpoint log message, I replaced "slru" with "SLRU" for consistency,
   as uppercase is typically used for SLRU.
- I updated the commit message.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  28. Re: Inconsistency in reporting checkpointer stats

    Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2024-10-01T16:50:06Z

    > Before committing the patch, I revised it with the following changes:
>
> - I added "shared" to the description of pg_stat_checkpointer.buffers_written
>   to clarify that it tracks shared buffers.
> - In the checkpoint log message, I replaced "slru" with "SLRU" for consistency,
>   as uppercase is typically used for SLRU.
> - I updated the commit message.

Thanks for updating. Looks good to me.

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft

On Tue, Oct 1, 2024 at 9:28 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2024/10/01 3:33, Fujii Masao wrote:
> >
> >
> > On 2024/09/22 20:44, Nitin Jadhav wrote:
> >>> +                                               CheckpointStats.ckpt_slru_written,
> >>> +                                               (double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
> >>>
> >>> I don't think NBuffers represents the maximum number of SLRU buffers.
> >>> We might need to calculate this based on specific GUC settings,
> >>> like transaction_buffers.
> >>
> >> Great observation. Since the SLRU buffers written during a checkpoint
> >> can include transaction_buffers, commit_timestamp_buffers,
> >> subtransaction_buffers, multixact_member_buffers,
> >> multixact_offset_buffers, and serializable_buffers, the total count of
> >> SLRU buffers should be the sum of all these types. We might need to
> >> introduce a global variable, such as total_slru_count, in the
> >> globals.c file to hold this sum. The num_slots variable in the
> >> SlruSharedData structure needs to be accessed from all types of SLRU
> >> and stored in total_slru_count. This can then be used during logging
> >> to calculate the percentage of SLRU buffers written. However, I’m
> >> unsure if this effort is justified. While it makes sense for normal
> >> buffers to display the percentage, the additional code required might
> >> not provide significant value to users. Therefore, I have removed this
> >> in the attached v6 patch.
> >
> > +1
> >
> > Thanks for updating the patch! It looks good to me.
> > Barring any objections, I will commit this patch.
>
> Before committing the patch, I revised it with the following changes:
>
> - I added "shared" to the description of pg_stat_checkpointer.buffers_written
>    to clarify that it tracks shared buffers.
> - In the checkpoint log message, I replaced "slru" with "SLRU" for consistency,
>    as uppercase is typically used for SLRU.
> - I updated the commit message.
>
> Regards,
>
> --
> Fujii Masao
> Advanced Computing Technology Center
> Research and Development Headquarters
> NTT DATA CORPORATION

  29. Re: Inconsistency in reporting checkpointer stats

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-10-02T02:21:50Z

    
On 2024/10/02 1:50, Nitin Jadhav wrote:
>> Before committing the patch, I revised it with the following changes:
>>
>> - I added "shared" to the description of pg_stat_checkpointer.buffers_written
>>    to clarify that it tracks shared buffers.
>> - In the checkpoint log message, I replaced "slru" with "SLRU" for consistency,
>>    as uppercase is typically used for SLRU.
>> - I updated the commit message.
> 
> Thanks for updating. Looks good to me.

Thanks for the review! Pushed.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION