On Fri, Oct 27, 2023 at 10:23:34AM +0530, Bharath Rupireddy wrote:
> A possible way is to move existing pgstat_count_slru_flush in
> SimpleLruWriteAll closer to pg_fsync and WAIT_EVENT_SLRU_SYNC in
> SlruPhysicalWritePage, remove WAIT_EVENT_SLRU_FLUSH_SYNC completely,
> use WAIT_EVENT_SLRU_SYNC in SlruSyncFileTag and count the flushes in
> SlruSyncFileTag. This aggregated way is much simpler IMV.
> 
> Another possible way is to have separate stat variables for each of
> the SLRU flushes WAIT_EVENT_SLRU_SYNC and WAIT_EVENT_SLRU_FLUSH_SYNC
> and expose them separately in pg_stat_slru. I don't like this
> approach.

This touches an area covered by a different patch, registered in this
commit fest as well:
https://www.postgresql.org/message-id/camm1awb18ept0whjrjg+-nyhnouxet6zuw0pnyyae+nezpv...@mail.gmail.com

So perhaps we'd better move the discussion there.  The patch posted
there is going to need a rebase anyway once the split with
pg_stat_checkpointer is introduced.

>> The error message generated for an incorrect target needs to be
>> updated in pg_stat_reset_shared():
>> =# select pg_stat_reset_shared('checkpointe');
>> ERROR:  22023: unrecognized reset target: "checkpointe"
>> HINT:  Target must be "archiver", "bgwriter", "io", "recovery_prefetch", or 
>> "wal".
> 
> +1. Changed in the attached v10-001. FWIW, having a test case in
> stats.sql emitting this error message and hint would have helped here.
> If okay, I can add one.
> 
> PS: I'll park the SLRU flush related patch aside until the approach is
> finalized. I'm posting the pg_stat_checkpointer patch as v10-0001.

Thanks.  That seems OK.  I don't have the wits to risk my weekend on
buildfarm failures if any, so that will have to wait a bit.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to