aws / amazon-sagemaker-examples

Example 📓 Jupyter notebooks that demonstrate how to build, train, and deploy machine learning models using 🧠 Amazon SageMaker.
https://sagemaker-examples.readthedocs.io
Apache License 2.0
10.04k stars 6.75k forks source link

The `serve` script in examples should use `sync`, not `gevent` workers #1561

Open empiricalthought opened 4 years ago

empiricalthought commented 4 years ago

Hi,

We spent a lot of time on resolving an issue with some code based on the examples here, and we determined that the line here was stopping our code from behaving correctly when deployed via Sagemaker:

https://github.com/awslabs/amazon-sagemaker-examples/blob/d6a25381975332aa79df61144fc0c2421bd5168c/advanced_functionality/scikit_bring_your_own/container/decision_trees/serve#L51

When we replaced this with sync our code behaved correctly. This link has information about what might be the trouble: Why You Should (Almost) Always Choose Sync Gunicorn Workers. Specifically,

Using Gevent needs all of your code to be co-operative in nature or all libraries you use should be monkey-patchable. Now this line may sound simple but it’s a big ask. This means either ensuring all the DB drivers, clients, 3rd party libraries used all of these are either pure python, which would ensure they can be monkey-patched OR they are written specifically with Gevent support.

Given that the intent of the container/Sagemaker approach is to enable deployment of code written by data scientists and other people who may not be experienced enough to vet their chosen libraries for proper behavior under arcane asynchronous execution circumstances, I suggest changing the scripts provided here to use the more-likely-to-be-correct default of a sync worker.

Thanks!

seanpmorgan commented 3 years ago

Thanks @empiricalthought! It also removes an unwieldy dependency in gevent. PR has been submitted

seanpmorgan commented 3 years ago

Opening this one again since there are several other examples that use gevent.