Add a simple implementation of max connections #44
Conversation
max limits the sum of active connections and listeners. The implementation assumes that max is greater than acceptor_pool_size, and always maintains the acceptor_pool at acceptor_pool_size as long as the max is not exceeded. If max is less than acceptor_pool_size, acceptor_pool_size is the effective max.
|
If you add tests, we'll review this for acceptance |
|
Thanks, I will. Do I take it that you are okay with the general approach of the commit? |
|
The general approach looks ok, but without good tests that I can run to verify the functionality it will be hard to accept. |
|
eunit tests added as requested. the tests rely on timers to ensure that connection setup completes and socket close events get noticed; so the tests are sensitive to the values of the timers. |
|
I'm definitely not fond of using timers in tests like this, it just means they'll either take longer than they need to, or they'll fail randomly when the machine running the test is slow. I'd like to have a different approach here before merging to master. For this kind of test we would typically use mocking, I would be ok to add meck as a mochiweb dependency in order to facilitate this (we use meck in basically all of our apps anyway). |
|
The main functionality of this patch appears to be isolated to recycle_acceptor which should be easily testable in isolation without the large scale and potentially fragile tests attached to this patch. recycle_acceptor takes 2 arguments, a pid whose liveness and pidliness are never checked, which means for the purposes of this test case it could be replaced with a well known atom… lets call it "dummy_acceptor_pid" and a State variable which is easily constructued. It also returns a new State structure which can be easily inspected for the correct behaviour of incrementing active sockets and transforming the acceptor pool. It's only side-effect is the starting of a mochiweb_acceptor processes which could be mocked using a library like meck. (Assuming etrepum doesn't have an issue adding meck as a dependency of mochiweb… it would certainly make a variety of testing easier.) We might lose coverage of some of the rest of mochiweb_socket_server not changed by this patch but I'd rather take a patch with correct test cases for the changes the patch makes than take a patch that adds exhaustive but fragile coverage. |
|
I'm willing to add meck as a dependency |
…yload." since this causes issues in the python ddfs client. This reverts commit c1d8910.
|
This pull request is outdated. I have created a new pull request with these commits from @pmundkur (Pull Request 126). |
max limits the sum of active connections and listeners.
The implementation assumes that max is greater than acceptor_pool_size,
and always maintains the acceptor_pool at acceptor_pool_size as long
as the max is not exceeded.
If max is less than acceptor_pool_size, acceptor_pool_size is the
effective max.