Date: Mon, 25 Nov 2024 07:12:56 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: per backend I/O statistics
Message-ID: <Z0QjeIkwC0HNI16K@ip-10-97-1-34.eu-west-3.compute.internal>
References: <ZzX7Y5i56UrOtlRC@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZzvuScIJW9kwnuyE@paquier.xyz>
 <Zzy8x7fGkWEKB7SF@ip-10-97-1-34.eu-west-3.compute.internal>
 <Zz0m1k03k5LA_c8S@paquier.xyz>
 <Zz3wIuQUJysX5O3d@ip-10-97-1-34.eu-west-3.compute.internal>
 <Zz5gMM9w59guO9S_@paquier.xyz>
 <Zz9sno+JJbWqdXhQ@ip-10-97-1-34.eu-west-3.compute.internal>
 <Zz_gHQ2g8MUe8oJ1@paquier.xyz>
 <Z0A3pmORCbhRe0Ug@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z0PNpAjhcS7S-1LI@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Z0PNpAjhcS7S-1LI@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/Z0QjeIkwC0HNI16K%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

Hi,

On Mon, Nov 25, 2024 at 10:06:44AM +0900, Michael Paquier wrote:
> On Fri, Nov 22, 2024 at 07:49:58AM +0000, Bertrand Drouvot wrote:
> > On Fri, Nov 22, 2024 at 10:36:29AM +0900, Michael Paquier wrote:
> >> Hmm.  created_entry only matters for pgstat_init_function_usage().
> >> All the other callers of pgstat_prep_pending_entry() pass a NULL
> >> value. 
> > 
> > I meant to say all the calls that passe "create" as true in pgstat_get_entry_ref().
> 
> Ah, OK, I think that I see your point here.
> 
> I am wondering how much this would matter as well for custom stats,
> but we're not there yet without at least one release out and folks try
> new things with these APIs and variable-numbered kinds.

Not sure here, could custom stats start incrementing before the database system
is ready to accept connections?

> pgstat_prep_pending_entry() to return NULL even if "create" is true
> may be a good thing, at the end, because that's the only way I can see
> based on the current APIs where we could say "Sorry, but the stats
> have not been loaded yet, so you cannot try to do anything related to
> the dshash".

Yeah, same here.

> From my view having a kind of barrier would be cleaner in the long
> run, but it's true that it may not be mandatory, as well.  pg_stat_io
> is currently OK to be called because the stats are loaded for
> auxiliary processes because it uses fixed-numbered stats in shmem.
> And it means we already have early calls that add stats getting
> overwritten once the stats are loaded from the on-disk file (Am I
> getting this part right?).

Yeah, we can already see that, for example, the background writer could enter 
pgstat_io_flush_cb() before the stats are reset or restored.

> Anyway, do we really require that for the sake of this thread?  We
> know that there's only one of each auxiliary process at a time, and
> they keep a footprint in pg_stat_io already.  So we could just limit
> outselves to live database backends, WAL senders and autovacuum
> workers, everything that's not auxiliary and spawned on request?

I think that's a fair starting point and that we will not lose any informations
doing so (as you said there is only one of each auxiliary process at a time,
so that one could already see their stats from pg_stat_io). 

The only cons that I can see is that we will not be able to merge the flush cb
but I don't think that's a blocker (the flush are done in shared memory so the
impact on performance should not be that much of an issue).

I'll come back with a new version implementing the above.

[1]: https://www.postgresql.org/message-id/Zz9sno%2BJJbWqdXhQ%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

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