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