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