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

Clear locks using reserve_sql_strategy #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

njakobsen
Copy link

When shutting down and releasing jobs, we should still respect the ActiveRecord.configuration.reserve_sql_strategy and clear locks using explicit job ids if the strategy is :default_sql. This can help avoid transaction isolation conflicts when shutting down several job runners.

When shutting down and releasing jobs, we should still respect the `ActiveRecord.configuration.reserve_sql_strategy` and clear locks using explicit job ids if the strategy is `:default_sql`. This can help avoid transaction isolation conflicts when shutting down several job runners.
@albus522
Copy link
Member

albus522 commented Oct 2, 2023

If locked_by is hitting a conflict you have misconfigured something and will have major issues with duplicate job runs.

@njakobsen
Copy link
Author

njakobsen commented Oct 2, 2023

It's not so much that locked_by is causing a conflict but that the non-primary key query to obtain a lock on a row uses a range lock, which must wait behind transactions that have created a job but have not committed yet. If those transactions are long running and plentiful, it can cause the delayed job shutdown process to take a long time. Therefore this change is intended to apply the same mechanism to unlocking as is used for locking.

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