Skip to content

Fix Rails optimistic locking when value in column is NULL for Issuables

We've had issues with optimistic locking in issues and MRs because our lock_version columns can contain NULLs. gitlab-com/gl-infra/production#821 (closed) is one of these, and there are some links from there to previous cases.

https://github.com/rails/rails/pull/26050 was intended to fix this, but didn't. @engwan pointed out a bug here: https://github.com/rails/rails/blob/v5.1.7/activerecord/lib/active_record/locking/optimistic.rb#L83

previous_lock_value = read_attribute_before_type_cast(locking_column)

This is reading the value from the attribute on the model, which is already dirtied. So if lock_version was NULL in the DB, and the user passed 0, this would return 0, not NULL. The update would then fail as the column doesn't have 0: https://github.com/rails/rails/blob/v5.1.7/activerecord/lib/active_record/locking/optimistic.rb#L91

UPDATE

This issue will track Issuables (Issues, Merge Requests, Epics) only, while #207306 (closed) has been created for the rest of the tables, since we want to wait until after running the required migrations to see the impact in the system to run the rest. We've also created #207312 (closed) to actually wrap this up after the migrations are done

Edited by Mario de la Ossa