From bd135d23c5c541c9ecfc3b370d01ec2d592c6553 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 13 Oct 2022 09:49:37 -0700 Subject: [PATCH] Nullify merge request and user associations in batches on user deletion We have been seeing failures to delete a user because the foreign key relation with `users.id` and `merge_requests.updated_by_id` is failing to nullify within the 15-second statement timeout. Just as we did with nullifying IDs in batches with the issues table in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84709, we need do the same with `merge_requests.updated_by_id` and `merge_requests.assignee_id`. Note that `author_id` and `merge_user_id` are assigned to the ghost user. Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/358760 Changelog: performance --- app/models/user.rb | 2 ++ spec/services/users/destroy_service_spec.rb | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index bb8d5d24b3b2f2..3230a18662039f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -197,6 +197,8 @@ def update_tracked_fields!(request) has_many :snippets, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent has_many :notes, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent has_many :issues, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent + has_many :legacy_assigned_merge_requests, class_name: 'MergeRequest', dependent: :nullify, foreign_key: :assignee_id # rubocop:disable Cop/ActiveRecordDependent + has_many :updated_merge_requests, class_name: 'MergeRequest', dependent: :nullify, foreign_key: :updated_by_id # rubocop:disable Cop/ActiveRecordDependent has_many :updated_issues, class_name: 'Issue', dependent: :nullify, foreign_key: :updated_by_id # rubocop:disable Cop/ActiveRecordDependent has_many :closed_issues, class_name: 'Issue', dependent: :nullify, foreign_key: :closed_by_id # rubocop:disable Cop/ActiveRecordDependent has_many :merge_requests, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index b32599d4af8970..c8ea7f704e2222 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -407,6 +407,21 @@ expect(issue.updated_by).to be_nil expect(resource_label_event.user).to be_nil end + + it 'nullifies assigned_merge_requests, last_updated_merge_requests' do + merge_request = create(:merge_request, source_project: project, target_project: project, + assignee: other_user, updated_by: other_user, merge_user: other_user) + + described_class.new(user).execute(other_user, skip_authorization: true) + + merge_request.reload + + aggregate_failures do + expect(merge_request.updated_by).to be_nil + expect(merge_request.assignee).to be_nil + expect(merge_request.assignee_id).to be_nil + end + end end end -- GitLab