Hi there,
I was looking at the ActiveRecord SQLite3 adapter and noticed a couple of issues. Two of the obvious ones I submitted pull requests for (ActiveRecord may leave statements open when using SQLite3 · Issue #13631 · rails/rails · GitHub and ActiveRecord+SQLite3 may roll back a transaction twice. · Issue #13638 · rails/rails · GitHub), however, one requires a bit more consideration: ActiveRecord + SQLite3 does not handle SQLite3::BusyException correctly within transactions · Issue #13908 · rails/rails · GitHub (or maybe should be closed).
The issue is that under certain circumstances (when there is a possibility of a dead lock), SQLite3 may return the SQLite3::BusyException immediately (even if a timeout is configured). ActiveRecord wraps the error into a StatementInvalid in such cases, as it is in the middle of a transaction. This is consistent with the documentation in ActiveRecord about the meaning of such errors:
104 # Warning: one should not catch ActiveRecord::StatementInvalid exceptions
105 # inside a transaction block. ActiveRecord::StatementInvalid exceptions indicate that an
106 # error occurred at the database level, for example when a unique constraint
107 # is violated. On some database systems, such as PostgreSQL, database errors
108 # inside a transaction cause the entire transaction to become unusable
109 # until it’s restarted from the beginning.
The transactions in SQLite3 on such errors, should also be rolled back and retried. All callers who are using AR with SQLite3, should then be wrapping their transactions with a begin/rescue/retry block in such cases. However, that seems a bit cumbersome. I was considering ways in which AR SQLite3 adapter could improve on that and one possibility is to have the adapter itself retry. I’m happy to submit a pull request for that, but not sure if that’s a direction that makes sense. That would be similar, it appears, to what one would have to do with PostrgreSQL (unfortunately, the SQLite3 issue comes up more often because of deferred transactions).
What do you guys think would be the best way to go about it (admittedly, leaving it be is also an option, but would require different code to be written depending on the adapters used).
Thank you,
Timur
