-
-
Couldn't load subscription status.
- Fork 406
Config web concurrency #4844
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
Config web concurrency #4844
Conversation
|
@zackkrida This is mergable, and doesn't harm the 1-click deployments, but I think it would make sense to document setting this variables for those scenarios as well. I'm not sure how the users should determine the number of vCPUs on various platforms, which is a prerequisite to setting this variable correctly. |
docker-compose.yml
Outdated
| # expose: | ||
| # - "5432" | ||
| ports: | ||
| - "5432:5432" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change necessary, I rememeber making a conscious decision to not expose the postgres port to the host.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mathemancer was this for testing purposes or some other reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! Great eye, @Anish9901 . That was for testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, reverted in 1682771
|
@mathemancer @zackkrida This commit cb2504f adds a default value for WEB_CONCURRENCY env variable for the from-scratch script based installation. |
Co-authored-by: Anish Umale <umaleanish120@gmail.com>
Fixes #4831
This adds a
WEB_CONCURRENCYenvironment variable to the docker compose setup. Appropriate settings are documented in the same file.TODO
Technical details
This variable is used by Gunicorn to set the number of workers, and is the only variable supported for changing concurrency settings, other than the variable to set all command-line arguments. The default I chose is "3", which is reasonable as long as Mathesar is running with at least 1 logical core available. This will improve the situation, but only a bit. Basically, with the
syncworker type, we need one worker per concurrent request. So, this takes us from 1 to 3 🎊 .Long-term, we should try to use the
geventworker type, since that enables async I/O, and Mathesar is extremely I/O, rather than CPU, bound. However, that doesn't work (easily) in Psycopg2, which means it doesn't affect Explorations. It does improve (fix, really) performance in the non-exploration part of the codebase, but I didn't want to make the change in the current limited-QA situation. If we decide we are not going to try to squeeze this PR into 0.6.0, then I think we should make that change as well, to get the benefits in the parts of the codebase where we're usingpsycopg3.This is yet another spot where our failure to deal with Explorations and the deprecated SQLAlchemy part of the codebase is biting us.
To test this, you can just load big tables in multiple browsers manually, or follow the instructions in #4841 .
Checklist
Update index.md).developbranch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin