Skip to content
This repository has been archived by the owner. It is now read-only.

Prevent a flood of connections keeping you out of the event loop #1057

Closed
wants to merge 4 commits into from

Conversation

@baudehlo
Copy link

baudehlo commented May 16, 2011

Uses a ramping up/down accept loop so that we don't get forced out of the event loop by a flood of connections.

@baudehlo

This comment has been minimized.

Copy link
Author

baudehlo commented May 17, 2011

Note: also applies with offsets against 0.4.7 and works well there.

@aaronblohowiak

This comment has been minimized.

Copy link

aaronblohowiak commented May 17, 2011

Do you have before/after benchmarks that show the influence on response time?

@baudehlo

This comment has been minimized.

Copy link
Author

baudehlo commented May 17, 2011

No, because it's very hard to trigger. I'll see what I can do with apachebench though.

@aaronblohowiak

This comment has been minimized.

Copy link

aaronblohowiak commented May 17, 2011

Ideally, you would use the same patched version of httperf as the article that this patch is in reply to.

@baudehlo

This comment has been minimized.

Copy link
Author

baudehlo commented May 17, 2011

Ideally, sure. But I'm on OSX, and it's pretty sucky there (apparently so is apachebench).

Honestly I've run into this before on other platforms (anyone writing event loop stuff has), and I know this fixes the problem. I welcome others to run benchmarks if they have access to a Linux box.

@ry

This comment has been minimized.

Copy link

ry commented May 27, 2011

Why can't we just have a hard limit - like 50?

@baudehlo

This comment has been minimized.

Copy link
Author

baudehlo commented May 27, 2011

This is just slightly fairer. Though I admit that would be simpler, but then it's not like it is complicated code.

@baudehlo

This comment has been minimized.

Copy link
Author

baudehlo commented May 27, 2011

I'd certainly be happy with a configurable exports.MAX_ACCEPT in 0.4.9.

Matt Sergeant added 2 commits Jun 8, 2011
@trevnorris

This comment has been minimized.

Copy link

trevnorris commented Nov 20, 2012

This pull request now has many conflicts. If it is to be merged, I'd say this should be closed and another pull request created with the updated code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.