rq / django-rq

A simple app that provides django integration for RQ (Redis Queue)
MIT License
1.81k stars 286 forks source link

RQScheduler django admin integration #587

Closed dmclain closed 1 year ago

dmclain commented 1 year ago

Cron/interval/repeat jobs managed by rq-scheduler can't presently be administered via django admin. This PR adds a couple new views that add the ability to view scheduled jobs including their next scheduled time Screenshot 2023-04-25 at 14 07 05

This PR has several open problems/decisions to resolve before considering merge:

selwin commented 1 year ago

Whoa, this is a huge effort. Thanks for this @dmclain .

There is a lot of code duplication that was done to try to behave somewhat consistently with what is present. Any specific code path or function that comes to mind? I'd love to try and streamline this piece by piece, we merge this into master while this branch continues being developed.

The choice to reference schedulers by queue index leads to some awkward results. As noted in the scheduler_stats view, rq-scheduler has a global list of scheduled jobs per redis connection. For users with multiple connections, referencing the connection information by queue makes some sense, but it results in every "queue" having the same number of jobs scheduled Since rq-scheduler is a global scheduler, it should not be displayed in this column (see screenshot). CleanShot 2023-05-02 at 08 17 25

We should have a dedicated RQ Scheduler section under the list of queues to show how many jobs are in RQ scheduler.

dmclain commented 1 year ago

I think listing rq-scheduler jobs in a separate section below the queues list makes sense.

In the screenshot the Scheduled Jobs listed is not an addition from this pull request, but rather a view into the ScheduledJobRegistry, which is a per-queue registry. It seems like rq-scheduler was largely merged into core rq, but the cron/interval/repeat functionailty was not brought in. I'd certainly like to have that functionality in rq and not have to run both manage.py rqworker --with-scheduler and manage.py rqscheduler), but that would be a larger pull request in other repos.

selwin commented 1 year ago

It seems like rq-scheduler was largely merged into core rq, but the cron/interval/repeat functionailty was not brought in.

Yes, I intentionally left out cron functionality because a Job instance is not designed to be run by two different workers simultaneously.

You could have a job that runs every minute, but each execution runs for longer than one minute, this case is not handled properly by RQ because execution state is saved on the job hash itself. ActiveJobRegistry (and others) are also not designed to handle concurrent executions of the same job.

To bring cron functionality into RQ, I think we'll need some sort of JobTemplate concept. Templates can be scheduled and cronned, when executed, it will spawn a new Job instance that can then be picked up by workers.

dmclain commented 1 year ago

Yes, I intentionally left out cron functionality because a Job instance is not designed to be run by two different workers simultaneously.

That makes sense, it's an edge case I hadn't considered.

After pulling the rq-scheduler information directly into the stats view, this PR became much cleaner.

Screenshot 2023-05-10 at 21 46 33
selwin commented 1 year ago

@dmclain I'm ok merging this in, let me know when you've tested https://github.com/rq/django-rq/pull/587#discussion_r1193058970 so we don't accidentally break this library for people using cron :)

dmclain commented 1 year ago

@selwin Apologies for the delay, I now have a test in the repo including the interval/repeat jobs that I was worried about before. I needed to add rq-scheduler to the pip install in the tests, but that seemed reasonable.

selwin commented 1 year ago

Thanks!