search

ninenines / ranch

Socket acceptor pool for TCP protocols.
ISC License
1.19k stars 336 forks source link

Stats counters for connection accepts and terminates #301

Closed Maria-12648430 closed 4 years ago

Maria-12648430 commented 4 years ago

As we discussed on IRC, here are stats counters for accepting and terminating connections.

The return of ranch:info will contain the rates under the stats key, as a tuple of the form {NumAccepts, NumTerminations}, in a map keyed with the ids of the connection supervisors.

Before I go any further (tests, documentation, ...), is this roughly as you imagine it or should I change something?

Maria-12648430 commented 4 years ago

Ah, I forgot to raise the OTP versions for the CI...

juhlig commented 4 years ago

Hm, I see a problem when it comes to release upgrades. There are two new fields in the state record of ranch_conns_sup, id and stats_counters_ref. When a release upgrade is performed, how do we find out what values to put in there?

essen commented 4 years ago

Hm, I see a problem when it comes to release upgrades. There are two new fields in the state record of ranch_conns_sup, id and stats_counters_ref. When a release upgrade is performed, how do we find out what values to put in there?

Perhaps in code_change we can do supervisor:which_children(Parent) and then look ourselves in the list to find the id which should be the same as the position I think? If not then supervisor:get_childspec/2 can be called to find the start arguments.

For stats_counter_ref I suspect the .appup will need to apply a function that creates them and then it can be retrieved in code_change by calling the appropriate function.

juhlig commented 4 years ago

I was thinking along the same lines, but I'm not sure the supervisor will be responsive as it might be suspended during the upgrade. Also, not sure self() will work in the conn_sup upgrade. We will have to try and see...

essen commented 4 years ago

self() definitely works but otherwise yes it might be suspended. If it is then we can probably query it before and save the value somewhere easily accessible.

Maria-12648430 commented 4 years ago

Sounds like some pain to me =^^= But I'll try.

juhlig commented 4 years ago

You can have a look at #284, it has an upgrade routine in ranch_conns_sup since it also introduces a new record field. It just puts in a default value, though, so it's smooth sailing with that one.

Maria-12648430 commented 4 years ago

Added a test and updated the documentation.

Maria-12648430 commented 4 years ago

Last commit adds release upgrade instructions, which pass the modified upgrade suite (#302).

Maria-12648430 commented 4 years ago

The last commit runs upgrade tests based on the modified upgrade suite (#302). It also tests that accepts and terminates are counted, but I'm not sure the way I did that is what you expect... Open for suggestions.

Maria-12648430 commented 4 years ago

Ok, MacOS again... Will fix that tomorrow...

Maria-12648430 commented 4 years ago

Hm, on closer inspection, that seems not to be my "fault"... It looks like that on MacOS, using the branch from #302, no upgrade happens at all.

@juhlig, please check that one again ;)

juhlig commented 4 years ago

@juhlig, please check that one again ;)

On it :)

essen commented 4 years ago

Thanks. If it's green we can merge.

Maria-12648430 commented 4 years ago

Green :)

essen commented 4 years ago

Merged, thanks!