On Fri, 2020-12-04 at 16:55 +0100, I wrote:
> > > Basically, that would change pgStatSessionDisconnectedNormally into instead being an
> > > enum of reasons, which could be normal disconnect, abnormal disconnect and admin.
> > > And we'd track all those three as separate numbers in the stats file, meaning we could
> > > then calculate the crash by subtracting all three from the total number of sessions?
> >
> > I think at least "closed by admin" might be interesting; I'll have a look.
> > I don't think we have to specifically count "closed by normal disconnect", because
> > that should be the rule and could be more or less deduced from the other numbers
> > (with the uncertainty mentioned above).
>
> 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 think I figured it out. Here is a patch along these lines.
I named the three counters "sessions_client_eof", "sessions_fatal" and
"sessions_killed", but I am not wedded to these bike shed colors.
Maybe we should, in honor of the bikeshed, we should call them sessions_blue, sessions_green etc :)
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?
Aside from that bikeshedding, I think this version looks very good!
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"?
+ 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++;
+ typedef enum sessionEndType {
To be consistent with the other enums in the same place, seems this should be SessionEndType.