Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use gunicorn #635

Closed
wants to merge 1 commit into from
Closed

Conversation

ryandawsonuk
Copy link
Contributor

@ryandawsonuk ryandawsonuk commented Jun 19, 2019

First steps towards addressing #383

See the link I posted on that issue. Seems to be common to use just flask on local machine and add gunicorn within container. See for example https://blog.miguelgrinberg.com/post/the-flask-mega-tutorial-part-xix-deployment-on-docker-containers

Have also done this in https://github.com/SeldonIO/seldon-core/blob/master/seldon-request-logger/Dockerfile

@axsaucedo
Copy link
Contributor

Just as a heads up it's worth taking into consideration that the Gunicorn server actually spins up multiple workers by default, which normally is indeed desired), howver given that we (at least currently) offload load balancing and replicas to the gateway components, we may have to avoid having more than one worker running. I do think that the benefits make it a great choice, and it's not too hard to set up 👍 we could even use this as an opportunity to discuss normalising the microservice vs seldonEngine APIs

@ukclivecox
Copy link
Contributor

We have considered gUnicorn as a standard way to wrap the Flask server we use for the python wrapper. This is also used by Sagemaker. Another option is to remove Flask and use Tornado.

@ryandawsonuk
Copy link
Contributor Author

My thinking is that gunicorn on k8s will help to get maximum use out of each container. The articles indicate that you can reliably handle a greater throughput of requests in a single container by adding gunicorn. So I’d expect that to carry over to multiple resource-constrained containers. To be sure we could do new benchmarking https://docs.seldon.io/projects/seldon-core/en/latest/reference/benchmarking.html

My impression is it doesn’t seem to negatively affect cold-start.

We could instead just add a note to the docs saying that gunicorn isn’t included by default and how to add it if desired.

@ryandawsonuk
Copy link
Contributor Author

Closing this in favour of #684

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants