Add support for EM.set_simultaneous_accept_count(<int>). #420

Merged
merged 1 commit into from Feb 8, 2015

Projects

None yet

5 participants

@macournoyer
Contributor

This controls the number of calls to accept(2) done in one turn in the loop. The default value is 10, like before, so it won't change the default behaviour of EM.

The purpose of this it to prevent EM from accepting more connections than it can handle.
My end goal is to set this to 1 in Thin.

Also add EM.get_simultaneous_accept_count to retreive the value.

If necessary, I can port my changes to the Java backend. Let me know.

/cc @raggi

@macournoyer macournoyer Add support for EM.set_simultaneous_accept_count(<int>).
Also add EM.get_simultaneous_accept_count to retreive the value.
This controls the number of calls to accept(2) done in one turn in the
loop.

The purpose of this it to prevent EM from accepting more connections
than it can handle.
9a4e95f
@macournoyer macournoyer referenced this pull request in macournoyer/thin Mar 14, 2013
Open

Thin 2 #167

0 of 27 tasks complete
@tehprofessor

I'd like to just chime in, and +1 one this commit being accepted...

@ibc
Contributor
ibc commented Mar 19, 2014

This commit will be ignored by EM developers, as are all the others. EM is dead, waste your efforts anywhere else ;)

@sodabrew sodabrew added this to the v1.0.7 milestone Feb 7, 2015
@sodabrew
Contributor
sodabrew commented Feb 7, 2015

@macournoyer Are you still pursuing Thin 2, and would this be of value even to Thin 1?

@raggi raggi merged commit 03224a6 into eventmachine:master Feb 8, 2015
@sodabrew
Contributor
sodabrew commented Feb 8, 2015

Ok! Actually I was going to suggest maybe calling this EM.accept_per_crank - and begin linking up a group of *crank options. Like the one for Rubinius cranking in Ruby instead of inside the EventMachine->Run() method.

@raggi
Contributor
raggi commented Feb 9, 2015

I proposed EM.crank() for single-step EM a long time ago (useful for tests, as you can also then do crank_until(timeout), etc). @tmm1 didn't like it.

I'm not really sure what this commit is expecting to change though, in terms of real world semantics. Modern OSes in default configurations will pre-accept anyway, and app pulling another 20 off a queue of 1024 (linux) won't make a huge difference in terms of balancing, etc. I could see this helping reduce dead letter reads by some proportional amount, and maybe it produces just enough backpressure to improve some semantics, but overall we'd need a lot more tunables to fix accept load balancing.

My best guess is that this reduces some of the failed connection traffic when overloading thin with a tool like wrk?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment