Fix IndexStatus find_or_create race condition
What does this MR do?
TL;DR Fix a race condition where IndexStatus
is not created and does not
raise error. Need to use !
and get rid of problematic unique model
validation.
Looking at the code I notice a logical problem with using
find_or_create_by
alongside a unique model validation. The problem is
a race condition which comes from find_or_create
first checking to see
if the record exists. If it does not it tries creating which then
invokes the validation which checks again to see if the record exists
which finally returns a validation error. But see here we are not using
!
version of find_or_create
and so this validation error is silently
ignored.
Since we already have a
safe_find_or_create_by!
which is designed for handling this race condition using unique indexes
and rescue and seeing that this model already has the unique
index
we should just get rid of the model validation since it just complicates
the error handling as we'd need a 2nd rescue/retry case to avoid missing these saves.
Since this is a pretty weird race condition it seems tricky/contrived to try to create a proper unit test for this case.
Screenshots
Does this MR meet the acceptance criteria?
Conformity
- [-] Changelog entry
- [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team