rq / django-rq

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

Feature/add scheduler pid info #570

Closed gabriels1234 closed 1 year ago

gabriels1234 commented 1 year ago

Based off @selwin encouragement (#567), This PR adds to the general view the PID of the scheduler. It handles both rq-scheduler and rq worker --with-scheduler.

it helps visualize whether there's an issue (e.g worker started without scheduler, or rq-scheduler not responding). Since worker-schedulers are queue-specific, it made a lot of sense to add it as a column to the main django-rq view.

It does that by checking scheduler data in redis (getting connection either from the queue or from the scheduler) and considers that rq and rq-scheduler store this differently.

If there's no active scheduler, the column will show "Inactive" (this might take some time since the data in redis has to expire (1~(rq) to 15~ (rq-scheduler) minutes)).

Wrote some tests, but need help in running them (rq has it easier). PS: (PIDs can be the same but the schedulers can be different (check the name of the worker to confirm they are different))

selwin commented 1 year ago

@gabriels1234 do you mind checking the tests and make sure they pass?

gabriels1234 commented 1 year ago

@gabriels1234 do you mind checking the tests and make sure they pass?

@selwin Thanks, made it so the tests work. 2 tests with --with-scheduler and 2 tests with RQ-Scheduler. (Tests passed via installing rq-scheduler, testing (2 passed, 2 skipped), then uninstalling rq-scheduler, testing (2 passed, 2 skipped))

gabriels1234 commented 1 year ago

@selwin let me know if the tests work for you. (I added 4 new tests, 2 (active/inactive) for --with-scheduler and 2 for rq-scheduler). Not sure if the other tests were working or not, but mine do work.

gabriels1234 commented 1 year ago

@selwin Based on the merged PR by dear @cunla (https://github.com/rq/rq/pull/1787), now that RQ proper has the scheduler_pid, should I use that and add it to the logic to get the pid when the scheduler is rq-scheduler?

cunla commented 1 year ago

@selwin Based on the merged PR by dear @cunla (rq/rq#1787), now that RQ proper has the scheduler_pid, should I use that and add it to the logic to get the pid when the scheduler is rq-scheduler?

It needs to be released prior to that. @selwin, when is it expected to be released?

selwin commented 1 year ago

@selwin Based on the merged PR by dear @cunla (https://github.com/rq/rq/pull/1787), now that RQ proper has the scheduler_pid, should I use that and add it to the logic to get the pid when the scheduler is rq-scheduler?

Let's just add a note that the current implementation should eventually be replaced with queue.scheduler_pid a few months after RQ 1.13 is released. Currently this library only requires RQ >= 1.2 so bumping it all the way up to require 1.13 is a bit too big of a jump. I'll move the minimum RQ requirement to 1.8 in the next release so we can slowly drop support for older RQ versions.

selwin commented 1 year ago

May I get a screenshot of how this looks like on a queue with scheduler active?

gabriels1234 commented 1 year ago

@selwin looking forward to hear from you how should I go about.

selwin commented 1 year ago

Sorry for the late reply @gabriels1234

gabriels1234 commented 1 year ago

Thanks!

image

Last column

selwin commented 1 year ago

Thanks!