Re: Inconsistency in reporting checkpointer stats

On Wed, Dec 21, 2022 at 5:15 PM Nitin Jadhav
<nitinjadhavpostgres(at)gmail(dot)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