Do not update Elasticsearch for pending_delete projects
What does this MR do?
Related to #262694 (closed)
Add an override method to the project_search
module to take into account pending_delete
setting for projects. We should not update Elasticsearch when a project is pending delete.
Background
We continue to see occasional errors in indexing Project
records into Elasticsearch on GitLab.com. The errors are due to the Project not having an associated Project Feature (which should be impossible). I finally got the additional data loading in Sentry (project_id field specifically) to see what other information I could find in the logs.
What I've seen is a pattern that indicates a race condition with the project being deleted, getting queued for indexing (via Elastic::ProcessBookkeepingService
), and being indexed in the middle of the deletion (where the error occurs). This data appears to be related to clean up of QA testing in staging and production. There is a QA::Tools::DeleteProjects
class which deletes projects by group and runs every 2 hours. I believe the error is occurring when:
-
delete_projects
calls the delete project API - API calls ::Projects::DestroyService async
- DestroyService updates the
pending_delete
attribute <-- this will kick off an update to Elasticsearch - DestroyService calls
ProjectDestroyWorker
async <-- race condition
Example 1
Sentry example: https://sentry.gitlab.net/gitlab/gitlabcom/issues/2613753/events/41666699/
Project id: 26500041
Correlation id: f0f89bdd9ae01cae56d1d285258debf7
Example 2
Sentry example: https://sentry.gitlab.net/gitlab/gitlabcom/issues/2613753/events/41595648/
Project id: 26490781
Correlation id: 70f3e12c0ee0c134786541c1f75f1eac
Example 3
https://sentry.gitlab.net/gitlab/gitlabcom/issues/2613753/events/41593604/
Project id: 26480892
Correlation id: 8c99afe20673fb4ce1a55a77233a4999
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Security
Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.
-
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