Re: Inconsistency in reporting checkpointer stats - Mailing list pgsql-hackers

> 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