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

Fix mysql deadlocks while reserving jobs #6

Merged
merged 1 commit into from
Aug 3, 2017

Conversation

ljfranklin
Copy link
Contributor

@ljfranklin ljfranklin force-pushed the PR-mysql-deadlocks branch from 4fa95cb to 6d03b16 Compare July 26, 2017 01:09
@ljfranklin
Copy link
Contributor Author

@JonathanTron gentle bump on this. Not sure if this repo is still maintained, if not we can keep our fork going. Thanks!

@JonathanTron
Copy link
Member

Hi @ljfranklin, thanks for reporting the issue and providing a PR.

We're not using MySQL at all, I'm willing to accept the PR, but could you do it like in the ActiveRecord version and only apply this fix if using MySQL?

- We frequently see deadlock errors when using mysql and a large number
  of workers
- This PR adds a fix for the deadlocks as well as modifying a test to
  exposes the deadlock issue in the previous implementation
- This implementation is based on the `reserve` implementation from the
  default active_record backend:
  https://github.com/collectiveidea/delayed_job_active_record/blob/42a68ed79917e14244a25a0a77fef0924bef3e97/lib/delayed/backend/active_record.rb#L64
- You can see the original error by pulling in the new test and running:
  `DEBUG=true DB=mysql bundle exec rspec spec/delayed/backend/sequel_spec.rb:27`
@ljfranklin
Copy link
Contributor Author

@JonathanTron sure, pushed that update. And if you're curious, here's an overly long thread about people arguing about deadlocks in the active_record backend: collectiveidea/delayed_job_active_record#63.

@JonathanTron
Copy link
Member

@ljfranklin Thanks a lot for the fast action!

@JonathanTron JonathanTron merged commit 13fe5c3 into TalentBox:master Aug 3, 2017
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.

2 participants