Re: Add session statistics to pg_stat_database - Mailing list pgsql-hackers

I've taken a look as well, and here are a few short notes:

* It talks about "number of connections" but "number of aborted sessions". We should probably be consistent about talking either about connections or sessions? In particular, connections seems wrong in this case, because it only starts counting after authentication is complete (since otherwise we send no stats)? (This goes for both docs and actual function names)

* Is there a reason we're counting active and idle in transaction (including aborted), but not fastpath? In particular, we seem to ignore fastpath -- if we don't want to single it out specifically, it should probably be included in active?

* pgstat_send_connstat() but pgstat_recv_connection(). Let's call both connstat or both connection (I'd vote connstat)?

* Is this actually a fix that's independent of the new stats? It seems in general to be changing the behaviour of "force", which is more generic?
-               !have_function_stats)
+               !have_function_stats && !force)

* in pgstat_send_connstat() you pass the parameter "force" in as "disconnect". That behaviour at least requires a comment saying why, I think. My understanding is it relies on that "force" means this is a "backend is shutting down", but that is not actually documented anywhere. Maybe the "force" parameter should actually be renamed to indicate this is really what it means, to avoid a future mistake in the area? But even with that, how does that turn into disconnect?

* Maybe rename pgStatSessionDisconnected to pgStatSessionNormalDisconnected? To avoid having to go back to the setting point and look it up in a comment.

I wonder if there would also be a way to count "sessions that crashed" as well. That is,the ones that failed in a way that caused the postmaster to restart the system. But that's information we'd have to send from the postmaster, but I'm actually unsure if we're "allowed" to send things to the stats collector from the postmaster. But I think it could be quite useful information to have. Maybe we can find some way to piggyback on the fact that we're restarting the stats collector as a result?