microsoft / planetary-computer-apis

Planetary Computer APIs
MIT License
105 stars 28 forks source link

STAC API and Tiler not using gunicorn worker and worker configuration #138

Open tonybaloney opened 1 year ago

tonybaloney commented 1 year ago

I just noticed that the Tiler API and STAC APIs are launching uvicorn directly to start FastAPI and not using gunicorn with a server worker.

This is fine for development, but for a production FastAPI instance it would create a queueing bottleneck for concurrent requests.

Secondly, there is no default worker processes and/or thread configuration for GUnicorn configured. This would significantly improvement throughput of the application (like a 4x improvement in requests/sec).

See https://www.uvicorn.org/deployment/#using-a-process-manager and https://fastapi.tiangolo.com/deployment/server-workers/

Spotted this in these places:

https://github.com/microsoft/planetary-computer-apis/blob/main/pctiler/Dockerfile#L47

https://github.com/microsoft/planetary-computer-apis/blob/main/pcstac/Dockerfile#L17

https://github.com/microsoft/planetary-computer-apis/blob/main/docker-compose.yml#L19

https://github.com/microsoft/planetary-computer-apis/blob/main/docker-compose.yml#L38

Happy to submit a PR with benchmark, unless I'm missing something

tonybaloney commented 1 year ago

For example,

instead of

uvicorn pctiler.main:app --host ${APP_HOST} --port ${APP_PORT} --log-level info

You would use something like

gunicorn pctiler.main:app --workers 4 --threads 4 --worker-class uvicorn.workers.UvicornWorker --bind ${APP_HOST}:${APP_PORT} --log-level info
vincentsarago commented 1 year ago

Thanks @tonybaloney

so in https://fastapi.tiangolo.com/deployment/server-workers/ it specifically says:

In particular, when running on Kubernetes you will probably not want to use Gunicorn and instead run a single Uvicorn process per container, but I'll tell you about it later in that chapter.

I think for Stac API we could relaxe this but for the tiler maybe not because it's really CPU intensive (and not really async)

tonybaloney commented 1 year ago

Ok that makes sense for the tiler.

For the STAC API, it depends on the concurrency potential of the requests and how Kubernetes is configured to scale-out the workers.

mmcfarland commented 1 year ago

Thanks @tonybaloney - we've been considering reverting back to GUnicorn for our STAC API to evaluate the impact on some behaviors we've been seeing with requests. In general, concurrency potential of those requests should be high. We'll want to do some benchmarking, but we appreciate the observations.