public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michael Paquier <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: per backend I/O statistics
Date: Mon, 16 Dec 2024 17:07:52 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<Z1rs/[email protected]>
	<[email protected]>
	<[email protected]>

On Fri, Dec 13, 2024 at 09:20:13AM +0000, Bertrand Drouvot wrote:
> Yeah makes sense, that's consistent with other kinds: done.

This looks to be taking shape.  I don't have much more comments.

Not feeling so sure about the value brought by the backend_type
returned in pg_stat_get_backend_io(), but well..

+	/* drop the backend stats entry */
+	pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber);

Oh, I've missed something.  Shouldn't pgstat_request_entry_refs_gc()
be called when this returns false?

The creation of the dshash entry is a bit too early, I think..  How
about delaying it more so as we don't create entries that could be
useless if we fail the last steps of authentication?  One spot would
be to delay the creation of the new entry at the end of
pgstat_bestart(), where we know that we are done with authentication
and that the backend is ready to report back to the client connected
to it.  It is true that some subsystems could produce stats as of the 
early transactions they generate, which is why pgstat_initialize() is
done that early in BaseInit(), but that's not really relevant for this
case?

I'm still feeling a bit uneasy about the drop done in
pgstat_shutdown_hook(); it would be nice to make sure that this
happens in a path that would run just after we're done with the
creation of the entry to limit windows where we have an entry but no
way to drop it, or vice-versa, so as the shutdown assertions would
never trigger.  Perhaps there's an argument for an entirely separate
callback that would run before pgstat is plugged off, like a new
before_shmem_exit() callback registered after the entry is created?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (12+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: per backend I/O statistics
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox