use current session count in session init callback

[dcrck]

gen_smtp_server_session's init/4 callback relies on the current session count, which should come from gen_smtp_server's sessions/1 function. This commit makes that change.

Ignore the last PR I submitted, that used the size/1 function instead of length/1 (I'm a bit unfamiliar with Erlang since I primarily use Elixir)

[seriyps]

But it's very inefficient...

[seriyps]

I mean, here is the code that is executed when you call gen_smtp_server:sessions():

https://github.com/ninenines/ranch/blob/4a1d16242c6fa9865fd7499d301d90b9ca65d6b1/src/ranch.erl#L440-L449

And it does multiple gen_server:call to Ranch supervisors and then does some nested list operations on the results. I intentionally dropped this functionality (providing the number of connections to init function) when we migrated from custom acceptor to ranch - because of performance concerns. My plan was to deprecate this init's 2nd argument eventually.

May I ask what is your usecase for it? Because if you want to use it for monitoring, it's better to periodically query ranch:get_connections(.., active) from your monitoring code. If you wantto limit the number of parallel connections, it's better to limit it using ranch options on start: gen_smtp_server:start(my_handler, my_callback,[{ranch_opts, #{max_connections => 256}}])

[dcrck]

hey @seriyps, thanks for reviewing. I didn't have a use case for this, but when I was reading through the code I noticed that there was a comment saying that this functionality needed "fixing". I guess that means deprecation, but that wasn't clear to me when I saw it. Plus, the session count is used in the example, so I figured it should make a relevant check.

Feel free to close this PR, but I'd recommend updating the example and the source code comment to note that the session count shouldn't be used because it's inefficient, or that you're planning to deprecate it.

[seriyps]

Thanks for the suggestions @dcrck ! I think I'll keep this PR as a reminder to implement what you suggested for now.

[Maria-12648430]

If you wantto limit the number of parallel connections, it's better to limit it using ranch options on start: gen_smtp_server:start(my_handler, my_callback,[{ranch_opts, #{max_connections => 256}}])

Note that the semantics/implications of max_connections have changed a bit between Ranch 1.x and 2.x. It is a soft limit, too, the actual number of connections might be slightly larger.

In general, it should be read as "maximum number of connections per connection supervisor".

Ranch 1.x only had 1 connection supervisor per listener, so the value given for max_connections indeed equals "maximum number of connections per listener" there.

Ranch 2.x, however, can have multiple connection supervisors per listener, given by the num_conns_sups option, which defaults to the value given in the num_acceptors option, which in turn defaults to 10. So, with max_connections set to 256 and without any other options, the maximum number of parallel connections in Ranch 2.x is really 2560.

(see https://ninenines.eu/docs/en/ranch/2.1/manual/ranch/)

[arjan]

I mean, here is the code that is executed when you call gen_smtp_server:sessions():

https://github.com/ninenines/ranch/blob/4a1d16242c6fa9865fd7499d301d90b9ca65d6b1/src/ranch.erl#L440-L449

And it does multiple gen_server:call to Ranch supervisors and then does some nested list operations on the results. I intentionally dropped this functionality (providing the number of connections to init function) when we migrated from custom acceptor to ranch - because of performance concerns. My plan was to deprecate this init's 2nd argument eventually.

May I ask what is your usecase for it? Because if you want to use it for monitoring, it's better to periodically query ranch:get_connections(.., active) from your monitoring code. If you wantto limit the number of parallel connections, it's better to limit it using ranch options on start: gen_smtp_server:start(my_handler, my_callback,[{ranch_opts, #{max_connections => 256}}])

Agreeing with @seriyps here, lets deprecate this init argument in the future.