From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: per backend I/O statistics |
Date: | 2024-09-13 16:09:40 |
Message-ID: | ZuRjxA+ysI4mpTSq@ip-10-97-1-34.eu-west-3.compute.internal |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(dot)pg(at)gmail(dot)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
Attachment | Content-Type | Size |
---|---|---|
v4-0001-per-backend-I-O-statistics.patch | text/x-diff | 30.6 KB |
v4-0002-Add-pg_stat_get_backend_io.patch | text/x-diff | 14.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | sia kc | 2024-09-13 16:27:48 | A starter task |
Previous Message | Nathan Bossart | 2024-09-13 15:21:21 | Re: Pgstattuple on Sequences: Seeking Community Feedback on Potential Patch |