Re: Inconsistency in reporting checkpointer stats

>  /* 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