Re: Add session statistics to pg_stat_database

On Sun, 2020-12-13 at 17:49 +0100, Magnus Hagander wrote:
> > > I am considering the cases
> > > 
> > > 1) client just went away (currently "aborted")
> > > 2) death by FATAL error
> > > 3) killed by the administrator (or shutdown)
> > 
> > I named the three counters "sessions_client_eof", "sessions_fatal" and
> > "sessions_killed", but I am not wedded to these bike shed colors.
> 
> In true bikeshedding mode, I'm not entirely happy with sessions_client_eof,
>  but I'm also not sure I have a better suggestion. Maybe just "sessions_lost"
>  or "sessions_connlost", which is basically the terminology that the documentation uses?
>  Maybe it's just me, but I don't really like the eof terminology here.
> 
> What do you think about that? Or does somebody else have an opinion here?

I slept over it, and came up with "sessions_abandoned".

> In today's dept of small things I noticed:
> 
> +   if (disconnect)
> +       msg.m_disconnect = pgStatSessionEndCause;
> 
> in the non-disconnect state, that variable is left uninitialized, isn't it?
> It does end up getting ignored later, but to be more future proof the enum should probably
>  have a value specifically for "not disconnected yet"?

Yes.  I named it DISCONNECT_NOT_YET.

> +       case DISCONNECT_CLIENT_EOF:
> +           ++(dbentry->n_sessions_client_eof);
> +           break;
> 
> The normal syntax we'd use for that would be
>   dbentry->n_sessions_client_eof++;

Ok, changed.

> + typedef enum sessionEndType {
> 
> To be consistent with the other enums in the same place, seems this should be SessionEndType.

True.  I have renamed the type.

Attached is patch version 9.
Added goodie: I ran pgindent on it.

Yours,
Laurenz Albe