Re: per backend I/O statistics

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-09-13T16:09:40Z
Lists: pgsql-hackers

Attachments

Hi,

On Fri, Sep 13, 2024 at 04:45:08PM +0300, Nazir Bilal Yavuz wrote:
> Hi,
> 
> Thanks for working on this!
> 
> Your patch applies and builds cleanly.

Thanks for looking at it!

> On Fri, 6 Sept 2024 at 18:03, Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > - As stated up-thread, the pg_stat_get_backend_io() behaves as if
> > stats_fetch_consistency is set to none (each execution re-fetches counters
> > from shared memory). Indeed, the snapshot is not build in each backend to copy
> > all the others backends stats, as 1/ there is no use case (there is no need to
> > get others backends I/O statistics while taking care of the stats_fetch_consistency)
> > and 2/ that could be memory expensive depending of the number of max connections.
> 
> I believe this is the correct approach.

Thanks for sharing your thoughts.

> I manually tested your patches, and they work as expected. Here is
> some feedback:
> 
> - The stats_reset column is NULL in both pg_my_stat_io and
> pg_stat_get_backend_io() until the first call to reset io statistics.
> While I'm not sure if it's necessary, it might be useful to display
> the more recent of the two times in the stats_reset column: the
> statistics reset time or the backend creation time.

I'm not sure about that as one can already get the backend "creation time"
through pg_stat_activity.backend_start.

> - The pgstat_reset_io_counter_internal() is called in the
> pgstat_shutdown_hook(). This causes the stats_reset column showing the
> termination time of the old backend when its proc num is reassigned to
> a new backend.

doh! Nice catch, thanks!

And also new backends that are not re-using a previous "existing" process slot
are getting the last reset time (if any). So the right place to fix this is in
pgstat_io_init_backend_cb(): done in v4 attached. v4 also sets the reset time to
0 in pgstat_shutdown_hook() (but that's not necessary though, that's just to be
right "conceptually" speaking).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Commits

  1. Rework handling of pending data for backend statistics

  2. Rename some pgstats callbacks related to flush of entries

  3. Relax regression test for fsync check of backend-level stats

  4. Add backend-level statistics to pgstats

  5. Extract logic filling pg_stat_get_io()'s tuplestore into its own routine

  6. Tweak some comments related to variable-numbered stats in pgstat.c