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

Stop passing --threads to gunicorn #1061

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

pjonsson
Copy link
Contributor

@pjonsson pjonsson commented Aug 12, 2024

This setting only applies to gthread
according to the documentation:

https://docs.gunicorn.org/en/23.0.0/settings.html#threads


📚 Documentation preview 📚: https://datacube-ows--1061.org.readthedocs.build/en/1061/

This setting only applies to gthread
according to the documentation:

https://docs.gunicorn.org/en/23.0.0/settings.html#threads
Copy link

codecov bot commented Aug 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.64%. Comparing base (8a86cd3) to head (aa05e6c).
Report is 23 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1061   +/-   ##
=======================================
  Coverage   93.64%   93.64%           
=======================================
  Files          43       43           
  Lines        6510     6510           
=======================================
  Hits         6096     6096           
  Misses        414      414           

@SpacemanPaul
Copy link
Contributor

SpacemanPaul commented Aug 12, 2024

We actually identified this as an issue about 6 months ago but promptly got bogged down in discussions over whether the correct fix was to remove the --threads argument or to switch to the gthread worker type.

In theory we should be MOSTLY threadsafe. AFAIA the only danger zone is our use of non-re-entrant matplotlib code when auto-generating legend images, which could be manually serialised with a thread lock.

I will use this PR to reinvigorate those discussions, thanks.

Edit: Actually may as well just merge this and open an issue for discussion.

@SpacemanPaul SpacemanPaul merged commit aa67e0b into opendatacube:master Aug 12, 2024
15 checks passed
@omad
Copy link
Member

omad commented Aug 12, 2024

Thank you Peter! I removed this foolishness from datacube-explorer, but forgot to follow up with OWS. 🤦🏻

@pjonsson pjonsson deleted the gunicorn-no-threads branch August 13, 2024 07:04
@SpacemanPaul SpacemanPaul mentioned this pull request Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants