Skip to content
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

Feature: Multiple Schedulers #212

Merged
merged 3 commits into from
Apr 24, 2020
Merged

Conversation

oxalorg
Copy link
Contributor

@oxalorg oxalorg commented Oct 11, 2019

This PR is in reference to issue #195

It adds the feature of running multiple scheduler instances (on multiple servers).

Each scheduler registers itself, then tries to acquire a lock to process the scheduling work. If the lock gets acquired it will schedule the jobs to the appropriate queues, remove the lock, and sleep for it's interval.

In the meanwhile another scheduler can try to acquire the lock. If it fails, it will simply sleep for its interval and try again later.

This way we don't have a single scheduler doing all the work. Different schedulers may acquire the lock at different time instances.

I've made sure that even those schedulers who can't acquire a lock have still registered their birth and a heartbeat makes sure the scheduler instance key doesn't expire even if the instance never acquires a lock.

Please let me know how we can proceed :)

@mattjegan
Copy link

@oxalorg this looks like it works well though for some reason when paired with django-rq it seems like the scheduler itself is executing the jobs rather than the rqworker process. My test was to run python manage.py rqscheduler --interval 10 twice and a worker with python manage.py rqworker. The worker process reported nothing however in the 2 ttys for the schedulers they printed out the debug statements from my test jobs.

@ArionMiles
Copy link

Hey @mattjegan I've created a dummy repository to test oxalorg's PR and I didn't come across what you experienced. The jobs are run by rqworker, no debug statements from the test jobs in my rqscheduler windows.

You can quickly pull and test this repo: https://github.com/ArionMiles/rqschedtest

Let me know if there's any other issues 😄

@mattjegan
Copy link

@ArionMiles Thanks for this, everything looks good with that repo. Perhaps it was something else in my env.

@ArionMiles
Copy link

Can you let me know what version of django you were testing on when you came across your problem? My test repo uses Django 1.11.

@ArionMiles
Copy link

If everything's okay with this PR, can we go ahead and merge this?

@kbuilds
Copy link

kbuilds commented Feb 17, 2020

Bllllluuuuurrrrrrrrrrrpppppppppppp (placing this useful note here so I get notified when this is merged)

@tadeoos
Copy link

tadeoos commented Feb 18, 2020

How are we doing here? We'd love to see this merged :)

@rassie
Copy link

rassie commented Apr 6, 2020

Another comment wishing for merge and asking for status. @oxalorg: could you rebase your branch? I'll be happy to point my pipenv to your fork for now.

@oxalorg
Copy link
Contributor Author

oxalorg commented Apr 6, 2020 via email

@rassie
Copy link

rassie commented Apr 6, 2020

Maybe, just maybe, the lock implementation should switch to redispy's Lock implementation, which uses Lua underneath to atomically extend lock's TTL.

@selwin
Copy link
Contributor

selwin commented Apr 24, 2020

Sorry it took so long for me to review this. Great PR!

@selwin selwin merged commit 0ef1bcb into rq:master Apr 24, 2020
@selwin
Copy link
Contributor

selwin commented Apr 24, 2020

One other thing, do you mind writing some docs about running multiple scheduler instances?

@kbuilds
Copy link

kbuilds commented Apr 24, 2020

🎉 🎉 🍾 🍾 🍷 🍻 🍺 🦃 🌵 🍆

@oxalorg
Copy link
Contributor Author

oxalorg commented Apr 24, 2020

One other thing, do you mind writing some docs about running multiple scheduler instances?

Thank you @selwin for merging. I'll send in another PR soon with the updated docs. 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants