Line 137 attempts to guard against concurrent modification of the record by wrapping the operation in a transaction. Unfortunately, MySQL has a default transaction isolation level of repeatable read, which is not strong enough to implement a proper compare-and-swap; concurrent changes are allowed to the database and will merely not be visible inside the transaction. Transactions with serializable snapshot isolation are needed to correctly implement a compare-and-swap operation. However, this level of isolation is likely not otherwise needed and has a performance penalty.
The appropriate solution here is to use locking reads when retrieving the record on line 164. Specifically, the query used here should be changed from SELECT nvalue FROM bucket WHERE nkey = ? to SELECT nvalue FROM bucket WHERE nkey = ? FOR UPDATE. This acquires an exclusive lock on the row immediately, and will block until it can acquire the lock if another transaction already has such a lock.
Shared locks (i.e. SELECT ... LOCK IN SHARE MODE) would potentially work here as well, but those handle concurrent operations in such a way that they're probably not the right choice. Multiple concurrent transactions can acquire shared locks on the same row but, depending on the timing, a deadlock may occur when one of them attempts to modify it, at which point one of the transactions will be rolled back. This would add a potential fail state to CmpAndSwap() independent of the record's value, which callers would need to handle. As such, this behavior is probably less desirable than that provided by an exclusive lock (i.e. SELECT ... FOR UPDATE).
As an aside, the error message on line 151 mentions badger instead of mysql.
The MySQL driver's implementation of the compare-and-swap operation does not correctly ensure that its operation is atomic.
https://github.com/smallstep/nosql/blob/4187cf9f7e66cad69963806f1740f340ce6c3f2d/mysql/mysql.go#L136-L177
Line 137 attempts to guard against concurrent modification of the record by wrapping the operation in a transaction. Unfortunately, MySQL has a default transaction isolation level of repeatable read, which is not strong enough to implement a proper compare-and-swap; concurrent changes are allowed to the database and will merely not be visible inside the transaction. Transactions with serializable snapshot isolation are needed to correctly implement a compare-and-swap operation. However, this level of isolation is likely not otherwise needed and has a performance penalty.
The appropriate solution here is to use locking reads when retrieving the record on line 164. Specifically, the query used here should be changed from
SELECT nvalue FROM bucket WHERE nkey = ?toSELECT nvalue FROM bucket WHERE nkey = ? FOR UPDATE. This acquires an exclusive lock on the row immediately, and will block until it can acquire the lock if another transaction already has such a lock.Shared locks (i.e.
SELECT ... LOCK IN SHARE MODE) would potentially work here as well, but those handle concurrent operations in such a way that they're probably not the right choice. Multiple concurrent transactions can acquire shared locks on the same row but, depending on the timing, a deadlock may occur when one of them attempts to modify it, at which point one of the transactions will be rolled back. This would add a potential fail state toCmpAndSwap()independent of the record's value, which callers would need to handle. As such, this behavior is probably less desirable than that provided by an exclusive lock (i.e.SELECT ... FOR UPDATE).As an aside, the error message on line 151 mentions
badgerinstead ofmysql.