Horton

Re: per backend WAL statistics

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org, Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: 2025-03-05T01:20:51Z
Lists: pgsql-hackers 
On Tue, Mar 04, 2025 at 08:48:28AM +0000, Bertrand Drouvot wrote:
> s/report/Report/ and s/system/system./? to be consistent with the other single
> line comments around.

Right.

> Yeah, I also think that's the right location.

We could be more optimal for the WAL receiver if we add more timestamp
calculations, I think, but that's a sensitive loop, and this is better
than no information anyway.  If somebody has a better idea, feel free.
We could have an extra GUC to control that, but I'm feeling that we
should restructure the WAL receiver before that, perhaps leverage some
of its activity elsewhere (?).

> +       /*
> +        * Some BackendTypes only perform IO under IOOBJECT_WAL, hence exclude all
> +        * rows for all the other objects for these.
> +        */
> +       if ((bktype == B_WAL_SUMMARIZER || bktype == B_WAL_RECEIVER ||
> +                bktype == B_WAL_WRITER) && io_object != IOOBJECT_WAL)
> +               return false;
> 
> I think that makes sense and it removes 15 lines out of 86. This function is
> "hard" to read/parse from my point of view. Maybe we could re-write it in a
> simpler way but that's outside the purpose of this thread. 

One thing I am planning to do here to improve the situation is the
addition of a regression test that queries pg_stat_io for all the
combinations of backend_type, object and contexts that are now
allowed, to keep track of the number of tuples we have.

> +   WHERE backend_type = 'walsummarizer' AND object = 'wal'});
> 
> The object = 'wal' is not needed (thanks to === 3), maybe we can remove this
> filtering?
> 
> Also what about adding a test to check that sum(reads) is NULL where object != 'wal'?

Not sure it matters as long as we track the supported combinations.
We need something a bit more general here.

(I've actually found a different issue while looking at the WAL
receiver, which is a bit older than what we have here.  Will post that
in a different thread with you in CC.)
--
Michael

Commits

  1. Fix use-after-free in pgstat_fetch_stat_backend_by_pid()

  2. Remove initialization from PendingBackendStats

  3. Add WAL data to backend statistics

  4. Improve check for detection of pending data in backend statistics

  5. Fix some gaps in pg_stat_io with WAL receiver and WAL summarizer

  6. Handle auxiliary processes in SQL functions of backend statistics

  7. Invent pgstat_fetch_stat_backend_by_pid()

  8. Refactor code of pg_stat_get_wal() building result tuple

  9. Adding new PgStat_WalCounters structure in pgstat.h

  10. Remove pgstat_flush_wal()

  11. Refactor some code related to backend statistics

  12. Add backend-level statistics to pgstats