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
signature.asc
Description: PGP signature
