On Mon, Mar 03, 2025 at 09:17:30AM +0000, Bertrand Drouvot wrote:
> On Mon, Mar 03, 2025 at 10:48:23AM +0900, Michael Paquier wrote:
>> Something that's still not quite right is that the WAL receiver and
>> the WAL summarizer do not call pgstat_report_wal() at all, so we don't
>> report much data and we expect these processes to run continuously.
>> The location where to report stats for the WAL summarizer is simple,
>> even if the system is aggressive with WAL this is never called more
>> than a couple of times per seconds, like the WAL writer:
>
> Same as above, that sounds right after a quick look.
Attached is a patch for this set of issues for the WAL receiver, the
WAL summarizer and the WAL writer. Another thing that we can do
better is restrict pgstat_tracks_io_object() so as we don't report
rows for non-WAL IOObject in the case of these three. Two tests are
added for the WAL receiver and WAL summarizer, checking that the stats
are gathered for both. For the WAL receiver, we have at least the
activity coming from one WAL segment created in the init context, at
least. The WAL summarizer is more pro-active with its reads in its
TAP test.
All that should be fixed before looking at the remaining patch for the
WAL stats at backend level, so what do you think about the attached?
>> I'm wondering if we should not lift more the list of processes listed
>> in pgstat_tracks_backend_bktype() and include B_AUTOVAC_LAUNCHER,
>> B_STARTUP, B_CHECKPOINTER, B_BG_WRITER at this stage, removing the
>> entire paragraph. Not sure if we really have to do that for this
>> release, but we could look at that separately.
>
> hm, do you mean update the comment on top of pgstat_tracks_backend_bktype() or
> update the documentation?
My argument would be to make pgstat_tracks_backend_bktype() the same
as pgstat_io.c, and reflect that in the docs and the comments.
> hmm, that would work as long as PGSTAT_BACKEND_FLUSH_ALL represents things
> that need to be called from pgstat_report_wal(). But I think that's open
> door for issue should be add a new PGSTAT_BACKEND_FLUSH_XXX where XXX is not
> related to pgstat_report_wal() at all. So, I'm tempted to keep it as it is.
OK, I can see your point here. Fine by me.
--
Michael
