python / asyncio

asyncio historical repository
https://docs.python.org/3/library/asyncio.html
1.04k stars 177 forks source link

Pause server #448

Open mayfield opened 7 years ago

mayfield commented 7 years ago

Re: https://groups.google.com/forum/?utm_source=digest&utm_medium=email#!topic/python-tulip/btGHbh5kUUM

Tests pass. I sort of wish the pause and resume functions could elegantly live on the event loop and not on the Server object, but it seems to be the best contextual storage for all the listening sockets.

Also I'm not sure max_connections on create_server is justifiable or even a good name, but it was immediately useful to my particular problem and I imagine it could be to others too. So I left it in.

1st1 commented 7 years ago

We'll need tests for this new functionality. I think that max_connections functionality is neat, but what's the use case again?

mayfield commented 7 years ago

@1st1 max_connections just says stop accepting new connections when the current active connection count is number. I have a proxy service that handles statsd and diamond data streams from thousands of clients, then adds some metadata and forwards to a wavefront agent. The ingress rate often exceeds the egress potential because of outside factors. Without a connection limit the server happily accepts new connections even while its egress rate plummets due to resource contention. My initial attempt to control backpressure with a semaphore on the recv side of the proxy wasn't good enough because I can't tell the event loop to stop accepting new connections. It only takes a few seconds to eat gigabytes of memory under the right conditions.

1st1 commented 7 years ago

But what criteria do you use to pick up a sound number for max_connections? Is it just some arbitrary number, or there is a way to calculate an optimal one for the given system?

mayfield commented 7 years ago

I'd expect it to be mostly application dependant. It would obviously need to be less than the nofile ulimit for the user running the process and then under the kernel's max open file limit and finally it needs to be a number that prevents memory exhaustion. The latter of which is going to vary depending on the memory footprint of each connection in a user's application.

In my particular case setting max_connections to 5000 puts my memory ceiling at ~ 100MB. But I actually get better performance with a lower number. Setting it too low however, can cause degraded service to clients. There are tradeoffs everywhere, hence a default of None (no limit).

mayfield commented 7 years ago

I'm writing some tests btw, but before I get too far, are you generally okay with the location of the functionality and the way the start_serving arguments are stored in Server? I frankly couldn't convince myself that I was using idiomatic conventions with that stuff. It's not clear to me when a user should interact with a Server object vs using event loop functions since most behavior is implemented in the event loop and the Server object seems to be just a placeholder for sockets and cleanup.

1st1 commented 7 years ago

I'm writing some tests btw, but before I get too far, are you generally okay with the location of the functionality and the way the start_serving arguments are stored in Server?

I think that the way the patch is stuctured now is fine. But before you get too far, let's wait until @gvanrossum gives this idea a green light.

mayfield commented 7 years ago

Should I be evangelizing this? I'd like to see it get accepted. @gvanrossum has publicly stated he's not focused on asyncio as much as he used to be, so perhaps there are other stakeholders we can bother?

1st1 commented 7 years ago

@gvanrossum What do you think on this one? 3.7?

gvanrossum commented 7 years ago

Definitely 3.7. Until then this could easily be implemented as a separate AbstractServer subclass.

mayfield commented 7 years ago

3.7 is fine, let me know how I can help or what I should do, if anything.

Re: subclassing AbstractServer, I believe I brought this up in the forum in the context of allowing a user to bring their own Server subclass to start_server but it was shot down. As it's architected now, I don't see how you can bring your own Server subclass without copypasta. But I may have missed something important.

1st1 commented 7 years ago

Re: subclassing AbstractServer, I believe I brought this up in the forum in the context of allowing a user to bring their own Server subclass to start_server but it was shot down. As it's architected now, I don't see how you can bring your own Server subclass without copypasta. But I may have missed something important.

@gvanrossum This is something we can still do in 3.6, right? IIUC we can add a server_factory parameter to loop.create_server?

gvanrossum commented 7 years ago

For something meant to go into the stdlib at 3.7, copypasta is probaby not a problem.

1st1 commented 7 years ago

Please reopen this PR at https://github.com/python/cpython. This repo is going to be deleted soon.