Replace Celery with RQ (except for execute_query tasks) #4093
Conversation
docker-compose.yml
Outdated
| build: . | ||
| command: dev_rq_worker | ||
| deploy: | ||
| replicas: 2 |
There was a problem hiding this comment.
Left this here for discussion - if one would want to allow multiple workers running simultaneously using docker-compose, one would have to run docker-compose --compatibility up
There was a problem hiding this comment.
Isn't deploy only for Docker Swarm deployments? Have you considered using scale option?
There was a problem hiding this comment.
If I understand correctly, the scale option is deprecated. One could use the --scale CLI option, and that would have the same consequence as using --compatibility. I just saw the deploy.replicas option as a better source of documentation for anyone who would want to scale through docker-compose.
There was a problem hiding this comment.
You're right. They deprecated it in v3 😒 I fail to see the logic behind this move, specially when they recommend not using --compatibility in production.
Considering we have no intention of using Swarm -- maybe we should switch to v2?
There was a problem hiding this comment.
We have at least one thing we need from v3.2 - bind volumes (for proper code reloading), so v2 is not an option. I guess it'll be best to recommend the --scale option outside this file. Where do you think would be the best place to do so?
There was a problem hiding this comment.
Do you remember what's the difference between how we define the volume for server and worker? I know you added it when working on auto restart, but from looking at the docs I fail to understand the difference.
If it is required, we can consider downgrading the production compose file to v2 as there the scale configuration is more important.
There was a problem hiding this comment.
server is using the default volume volume type, because Flask's auto reloading works fine with it. For worker (or anything that we use watchmedo for) we need a bind volume type for FSEvents to play nicely with macOS.
We could definitely use v2 for production compose files though.
There was a problem hiding this comment.
Ok, as this is all interim -- let's move on and revisit before migrating everything to RQ.
There was a problem hiding this comment.
OK, for now I've removed the deploy.replicas key.
| def worker(queues='default'): | ||
| with Connection(redis_connection): | ||
| w = Worker(queues) | ||
| w.work() |
There was a problem hiding this comment.
I should note that there is flask-rq2 which provides a Flask friendly API for this.
…heduler entrypoint
…ia get_task_logger for now)
…le using docker-compose would be the --scale CLI option, which will be described in the knowledge based
…bs to be scheduled after app has been loaded
…expiring and having periodic jobs not reschedule
|
@NicolasLM We're planning on merging this soon into master. Anything we can do to make your life easier post the merge? |
Thank you for the heads up. I don't expect much trouble with running this code on Python 3, the bulk of the work will probably be resolving conflicts. |
|
When we merge this we should post an update on Development mentioning that master is in a hybrid state at the moment and not recommended for general consumption. |
| cleanup_query_results, | ||
| version_check, send_aggregated_errors) | ||
|
|
||
| rq_scheduler = Scheduler(connection=redis_connection, |
There was a problem hiding this comment.
In Python 3 we use decode_responses=True, while rq-scheduler expects it to be False and uses decode on values it receives:
When porting this to Python 3, we should probably create a Redis connection for it without the decode_responses change 🤔 We can create a "factory" function for creating the Redis connection and move the logic of mangling the Redis URL there (and make it optional).
Another option to consider is to revert to not decoding responses and do it where needed. It just felt redundant, so I was hoping to avoid it.
What type of PR is this? (check all applicable)
Description
As a first probe towards migrating to RQ, I'm interested in trying to swap out Celery beat and all non-execute-query tasks and replace them with RQ.
Related Tickets & Documents
Towards #4092