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

Stop retrying semaphore decrement when getting a deadlock #152

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

rosa
Copy link
Member

@rosa rosa commented Feb 13, 2024

This was added in d72fe46 and initially, it seemed like a good idea, but it ended up causing a different error as it messed with Active Record's transactions. The error would be

ActiveRecord::StatementInvalid (Mysql2::Error: SAVEPOINT active_record_1 does not exist)

And this made us realise it might be more complicated, because we might be in a transaction that wraps the Solid Queue transaction.

Since this is such an edge case that happens only with certain job loads and perhaps only with MySQL, InnoDB and READ COMMITTED, let's stop trying to be too clever about it, and instead, simplify this a bit by avoiding the decrement at all when the semaphore's limit is 1 and it was just created.

This was added in d72fe46 and initially,
it seemed like a good idea, but it ended up causing a different error
as it messed with Active Record's transactions. The error would be
```
ActiveRecord::StatementInvalid (Mysql2::Error: SAVEPOINT active_record_1 does not exist)
```
And this made us realise it might be more complicated, because we might be
in a transaction that wraps the Solid Queue transaction.

Since this is such an edge case that happens only with certain job loads
and perhaps only with MySQL, InnoDB and READ COMMITTED, let's stop trying to
be too clever about it, and instead, simplify this a bit by avoiding the
decrement at all when the semaphore's limit is 1 and it was just created.
@rosa rosa merged commit d71aa8a into main Feb 21, 2024
6 checks passed
@rosa rosa deleted the dont-retry-deadlocks-on-semaphore-claim branch February 21, 2024 12:16
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.

None yet

1 participant