Commit f934106b authored by Pavel Shutsin's avatar Pavel Shutsin

Nullify dependent associations in batches on user deletion

Nullify related associations in small portions to
avoid query timouts for users with lots of activity
and data in DB.

Changelog: added
parent 3de4814e
# frozen_string_literal: true
# Provides a way to execute nullify behaviour in batches
# to avoid query timeouts for really big tables
# Assumes that associations have `dependent: :nullify` statement
module BatchNullifyDependentAssociations
extend ActiveSupport::Concern
class_methods do
def dependent_associations_to_nullify
reflect_on_all_associations(:has_many).select { |assoc| assoc.options[:dependent] == :nullify }
end
end
def nullify_dependent_associations_in_batches(exclude: [], batch_size: 100)
self.class.dependent_associations_to_nullify.each do |association|
next if association.name.in?(exclude)
loop do
# rubocop:disable GitlabSecurity/PublicSend
update_count = public_send(association.name).limit(batch_size).update_all(association.foreign_key => nil)
# rubocop:enable GitlabSecurity/PublicSend
break if update_count < batch_size
end
end
end
end
...@@ -21,6 +21,7 @@ class User < ApplicationRecord ...@@ -21,6 +21,7 @@ class User < ApplicationRecord
include OptionallySearch include OptionallySearch
include FromUnion include FromUnion
include BatchDestroyDependentAssociations include BatchDestroyDependentAssociations
include BatchNullifyDependentAssociations
include HasUniqueInternalUsers include HasUniqueInternalUsers
include IgnorableColumns include IgnorableColumns
include UpdateHighestRole include UpdateHighestRole
...@@ -189,6 +190,8 @@ class User < ApplicationRecord ...@@ -189,6 +190,8 @@ class User < ApplicationRecord
has_many :snippets, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent 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 :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 :issues, dependent: :destroy, foreign_key: :author_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 has_many :merge_requests, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
has_many :events, dependent: :delete_all, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent has_many :events, dependent: :delete_all, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
has_many :releases, dependent: :nullify, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent has_many :releases, dependent: :nullify, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
......
...@@ -64,6 +64,10 @@ module Users ...@@ -64,6 +64,10 @@ module Users
# This ensures we delete records in batches. # This ensures we delete records in batches.
user.destroy_dependent_associations_in_batches(exclude: [:snippets]) user.destroy_dependent_associations_in_batches(exclude: [:snippets])
if Feature.enabled?(:nullify_in_batches_on_user_deletion, default_enabled: :yaml)
user.nullify_dependent_associations_in_batches
end
# Destroy the namespace after destroying the user since certain methods may depend on the namespace existing # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing
user_data = user.destroy user_data = user.destroy
namespace.destroy namespace.destroy
......
...@@ -100,9 +100,9 @@ module Users ...@@ -100,9 +100,9 @@ module Users
end end
# rubocop:disable CodeReuse/ActiveRecord # rubocop:disable CodeReuse/ActiveRecord
def batched_migrate(base_scope, column) def batched_migrate(base_scope, column, batch_size: 50)
loop do loop do
update_count = base_scope.where(column => user.id).limit(100).update_all(column => ghost_user.id) update_count = base_scope.where(column => user.id).limit(batch_size).update_all(column => ghost_user.id)
break if update_count == 0 break if update_count == 0
end end
end end
......
---
name: 'nullify_in_batches_on_user_deletion'
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84709
rollout_issue_url:
milestone: '14.10'
type: development
group: group::optimize
default_enabled: true
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe BatchNullifyDependentAssociations do
before do
test_user = Class.new(ActiveRecord::Base) do
self.table_name = 'users'
has_many :closed_issues, foreign_key: :closed_by_id, class_name: 'Issue', dependent: :nullify
has_many :issues, foreign_key: :author_id, class_name: 'Issue', dependent: :nullify
has_many :updated_issues, foreign_key: :updated_by_id, class_name: 'Issue'
include BatchNullifyDependentAssociations
end
stub_const('TestUser', test_user)
end
describe '.dependent_associations_to_nullify' do
it 'returns only associations with `dependent: :nullify` associations' do
expect(TestUser.dependent_associations_to_nullify.map(&:name)).to match_array([:closed_issues, :issues])
end
end
describe '#nullify_dependent_associations_in_batches' do
let_it_be(:user) { create(:user) }
let_it_be(:updated_by_issue) { create(:issue, updated_by: user) }
before do
create(:issue, closed_by: user)
create(:issue, closed_by: user)
end
it 'nullifies multiple settings' do
expect do
test_user = TestUser.find(user.id)
test_user.nullify_dependent_associations_in_batches
end.to change { Issue.where(closed_by_id: user.id).count }.by(-2)
end
it 'excludes associations' do
expect do
test_user = TestUser.find(user.id)
test_user.nullify_dependent_associations_in_batches(exclude: [:closed_issues])
end.not_to change { Issue.where(closed_by_id: user.id).count }
end
end
end
...@@ -332,4 +332,39 @@ RSpec.describe Users::DestroyService do ...@@ -332,4 +332,39 @@ RSpec.describe Users::DestroyService do
expect(User.exists?(other_user.id)).to be(false) expect(User.exists?(other_user.id)).to be(false)
end end
end end
context 'batched nullify' do
let(:other_user) { create(:user) }
context 'when :nullify_in_batches_on_user_deletion feature flag is enabled' do
it 'nullifies related associations in batches' do
expect(other_user).to receive(:nullify_dependent_associations_in_batches).and_call_original
described_class.new(user).execute(other_user, skip_authorization: true)
end
it 'nullifies last_updated_issues and closed_issues' do
issue = create(:issue, closed_by: other_user, updated_by: other_user)
described_class.new(user).execute(other_user, skip_authorization: true)
issue.reload
expect(issue.closed_by).to be_nil
expect(issue.updated_by).to be_nil
end
end
context 'when :nullify_in_batches_on_user_deletion feature flag is disabled' do
before do
stub_feature_flags(nullify_in_batches_on_user_deletion: false)
end
it 'does not use batching' do
expect(other_user).not_to receive(:nullify_dependent_associations_in_batches)
described_class.new(user).execute(other_user, skip_authorization: true)
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment