Defect #39437
closed
MySQL / MariaDB issue nested set deadlocks and consistency
0%
Description
This relates to #19344 #19395 #23318 and #35014.
I did some investigation into this topic for Planio and thought I'd share my findings.
All tests were done on Linux, with Ruby 3.1 and Redmine master, databases (MySQL 5.6, 5.7 and MariaDB 11) in Docker.
To fix the deadlocks, I added a global lock, basically serializing all nested set modifications. I chose to useselect id from settings for update for this because it's a relatively static and small table, but in the
end it could be anything. The changed locking SQL from #23318-18 alone still resulted in deadlocks 100% of the time
in my setup, but since it was determined to be faster back then and the necessary mysql-specific branch was in the code now I added it anyway.
This is certainly neither elegant nor ideal, however I wanted to get around the deadlocks to reproduce another problem we were
seeing in the wild - corrupted nested sets, which now indeed happened along with stale record errors with every test run on all three
databases.
In my understanding, this is because of MySQLs default transaction isolation level, which is REPEATABLE READ (many
(most?) other commonly used RDBMS default to READ COMMITTED). The relevant difference between the two is, that in the
latter your transaction "sees" changes committed by other transactions as soon as they are COMMITed, while with REPEATABLE READ,
you continue to work on a snapshot that was created once, at the beginning of your transaction.
See i.e. https://mariadb.com/kb/en/set-transaction/#isolation-levels for reference.
In our case (concurrent modifications to the issue nested set, i.e. by parallel modification of various issues'
parent_id), this means:
- the transaction is implicitly opened by Rails when
Issue#saveis called - the nested set is locked at a later point in time (in an after_save hook)
- this time gap between snapshot creation (begin of transaction) and actual locking leaves room for race conditions.
A failing sequence of events might look like this (assuming the modified issues are part of the same (larger)
nested set):
- A starts transaction to update issue X
- B starts transaction to update issue Y
- several SELECTs are made by A and B (during validations etc), each now work on their own snapshot, each
not seeing modifications made by the other - B locks the nested set
- A attempts to lock the same, has to wait
- B modifies the set and commits, giving up the locks.
- A now gets the lock, but still has the old snapshot, so it doesn't see the changes made by B
- A now works on the set with stale data, which may lead to a corrupted tree and stale record errors
To fix this, the second patch adds an initializer that sets the tx isolation to "READ COMMITTED" by hooking into
the MySQL2 adaptor. With this in place, all tests now pass all the time, with MySQL 5.6, 5.7 and MariaDB 11.
Now, I am not 100% sure if we should put either of these patches into Redmine, but since it's, right now, not easy to work
around the deadlocks by retrying the transaction (that would mean retrying the Issue#save, which may or may not be free of
unwanted side effects), I do not see many alternatives if we want to fix this. Maybe we introduce a dedicated table
with a dedicated row that serves as the mutex to make it look a bit less "hacky".
Relaxing the TX isolation level might have side effects (although none appear to be discovered by our current test suite),
and one might consider it bad practice to do so at the application level, at all. So probably this initializer and the
reasoning behind it should just go into the Wiki as part of MySQL/MariaDB specific setup instructions. After all, the same
effect can be achieved by simply setting the default TX isolation level in the database server config. In theory, switching
to a lower tx isolation level should help reduce the probability of deadlocks, but this alone at least did not make the
concurrency test pass in my setup.
Another possible solution / workaround would be to move the whole nested set manipulation to a separate transaction.
This could be done by moving it into an after_commit hook and opening a new transaction in which the snapshot is
created with the SELECT ... FOR UPDATE. This way it should work regardless of the configured TX isolation level.
With such a separate (and much smaller) transaction, any deadlocks might also easily be handled by retrying the transaction,
instead of using the global lock workaround I introduced above.
Files
Related issues
Updated by 
Updated by 
Updated by 
Updated by