-
Notifications
You must be signed in to change notification settings - Fork 6
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
BC-269 Configure celery pool type and concurrency number. #201
base: master
Are you sure you want to change the base?
Conversation
Makefile
Outdated
@@ -58,7 +58,7 @@ run-server: | |||
|
|||
.PHONY: run-celery | |||
run-celery: | |||
poetry run celery -A node.config.celery worker --loglevel=INFO | |||
poetry run celery -A node.config.celery worker --loglevel=INFO --pool=eventlet --concurrency=30 |
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.
@olegtropinin why eventlet?
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.
@olegtropinin let's go with prefork or process and autoscale for now, we will optimize with async approach later if needed
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.
actually, these comments apply to docker-compose.yml only. here we need just keep it as it was, because it is OK for development
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.
I found information that "eventlet" pool type have greater concurrency than prefork. Also it's good for API calls. It was a case why I selected prefork.
I removed "eventlet" as you said. It has "prefork" default pool now.
You are right no need to have too many threads for development. I removed concurrency argument from Makefile (make run-celery).
docker-compose.yml
Outdated
@@ -47,7 +47,7 @@ services: | |||
TNB_DATABASES: '{"default":{"CLIENT":{"host":"node-mongo","password":"${MONGO_INITDB_ROOT_PASSWORD}"}}}' | |||
TNB_CELERY_BROKER_URL: 'amqp://guest:guest@celery-broker:5672//' | |||
env_file: .env | |||
command: poetry run celery -A node.config.celery worker --loglevel=INFO | |||
command: poetry run celery -A node.config.celery worker --loglevel=INFO --pool=eventlet --concurrency=30 |
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.
let's have concurrency depend on max PV schedule length (read from env var that corresponds to django setting)
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.
Concurrency value is now same as TNB_NODE_SCHEDULE_CAPACITY environment value.
dbb6bb7
to
9660f45
Compare
No description provided.