From 68dca76235bd7de0654751185e32a1db43c0bdee Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 14 Oct 2022 00:45:53 -0700 Subject: [PATCH] Nullify merge request metrics user in batches on user deletion We have been seeing failures to delete a user because the foreign key relation with `merge_request_metrics.merged_by_id` and `users.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_request_metrics.merged_by_id` and `merge_request_metrics.latest_closed_by_id`. Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/358760 Changelog: performance --- app/models/user.rb | 2 ++ spec/services/users/destroy_service_spec.rb | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 4bc2d8f76aa7f0..b2f75959f15b0d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -199,6 +199,8 @@ def update_tracked_fields!(request) 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 :merged_merge_requests, class_name: 'MergeRequest::Metrics', dependent: :nullify, foreign_key: :merged_by_id # rubocop:disable Cop/ActiveRecordDependent + has_many :closed_merge_requests, class_name: 'MergeRequest::Metrics', dependent: :nullify, foreign_key: :latest_closed_by_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 diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index c8ea7f704e2222..efaac06c1f28e9 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -408,9 +408,10 @@ expect(resource_label_event.user).to be_nil end - it 'nullifies assigned_merge_requests, last_updated_merge_requests' do + it 'nullifies merge request associations' do merge_request = create(:merge_request, source_project: project, target_project: project, assignee: other_user, updated_by: other_user, merge_user: other_user) + merge_request.metrics.update!(merged_by: other_user, latest_closed_by: other_user) described_class.new(user).execute(other_user, skip_authorization: true) @@ -420,6 +421,8 @@ expect(merge_request.updated_by).to be_nil expect(merge_request.assignee).to be_nil expect(merge_request.assignee_id).to be_nil + expect(merge_request.metrics.merged_by).to be_nil + expect(merge_request.metrics.latest_closed_by).to be_nil end end end -- GitLab