Re: per backend I/O statistics

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