diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 12d7ed15d00aef3ab542767d8ef49f8c9fac5c3a..403f7d69bc1346544de0db3f8108fa48d718d198 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -212,18 +212,44 @@ def log_destroy_event def destroy_mr_diff_relations! delete_batch_size = 1000 - project.merge_requests.each_batch(column: :iid, of: BATCH_SIZE) do |relation_ids| - loop do - inner_query = MergeRequestDiffFile - .select(:merge_request_diff_id, :relative_order) - .where(merge_request_diff_id: MergeRequestDiff.where(merge_request_id: relation_ids).select(:id)) - .limit(delete_batch_size) - - deleted_rows = MergeRequestDiffFile - .where("(merge_request_diff_files.merge_request_diff_id, merge_request_diff_files.relative_order) IN (?)", inner_query) - .delete_all - - break if deleted_rows == 0 + if Feature.enabled?(:merge_request_diff_commits_partition, project) + # partition compatible queries + project.merge_requests.each_batch(column: :iid, of: BATCH_SIZE) do |relation_ids| + loop do + inner_query = MergeRequestDiffCommit + .select(:merge_request_diff_id, :relative_order) + .where( + merge_request_diff_id: MergeRequestDiff.where(merge_request_id: relation_ids).select(:id) + ) + .where(project_id: project.id) + .limit(delete_batch_size) + + deleted_rows = MergeRequestDiffCommit + .where(project_id: project.id) + .where( + "(merge_request_diff_commits.merge_request_diff_id, merge_request_diff_commits.relative_order) IN (?)", + inner_query + ) + .delete_all + + break if deleted_rows == 0 + end + end + else + # records without project ids + project.merge_requests.each_batch(column: :iid, of: BATCH_SIZE) do |relation_ids| + loop do + inner_query = MergeRequestDiffFile + .select(:merge_request_diff_id, :relative_order) + .where(merge_request_diff_id: MergeRequestDiff.where(merge_request_id: relation_ids).select(:id)) + .limit(delete_batch_size) + + deleted_rows = MergeRequestDiffFile + .where("(merge_request_diff_files.merge_request_diff_id, merge_request_diff_files.relative_order) IN (?)", inner_query) + .delete_all + + break if deleted_rows == 0 + end end end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 34e78907b559fb7e2833cec6838ae026f95bd645..a4655c5348b465a1c6607beb4a2377b9c10e3aae 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -1087,6 +1087,62 @@ end end + describe '#destroy_mr_diff_relations!' do + let(:service) { described_class.new(project, user, {}) } + + context 'when merge_request_diff_commits_partition feature flag is enabled' do + let!(:merge_request) { create(:merge_request, source_project: project) } + let!(:merge_request_diff) { merge_request.merge_request_diffs.first } + + before do + MergeRequestDiffCommit.where(merge_request_diff_id: merge_request_diff.id).delete_all + + 3.times do |i| + MergeRequestDiffCommit.create!( + merge_request_diff_id: merge_request_diff.id, + project_id: project.id, + relative_order: i, + sha: Digest::SHA256.hexdigest("commit-#{i}"), + authored_date: Time.current, + committed_date: Time.current + ) + end + end + + it 'deletes MergeRequestDiffCommit records using partition-compatible queries' do + expect(MergeRequestDiffCommit.where(project_id: project.id).count).to eq(3) + + service.send(:destroy_mr_diff_relations!) + + expect(MergeRequestDiffCommit.where(project_id: project.id).count).to eq(0) + end + + it 'only deletes records for the current project' do + other_project = create(:project, :repository, namespace: user.namespace) + other_merge_request = create(:merge_request, source_project: other_project) + other_diff = other_merge_request.merge_request_diffs.first + + MergeRequestDiffCommit.where(merge_request_diff_id: other_diff.id).delete_all + + 2.times do |i| + MergeRequestDiffCommit.create!( + merge_request_diff_id: other_diff.id, + project_id: other_project.id, + relative_order: i, + sha: Digest::SHA256.hexdigest("other-commit-#{i}"), + authored_date: Time.current, + committed_date: Time.current + ) + end + + service.send(:destroy_mr_diff_relations!) + + expect(MergeRequestDiffCommit.where(project_id: project.id).count).to eq(0) + expect(MergeRequestDiffCommit.where(project_id: other_project.id).count).to eq(2) + end + end + end + def destroy_project(project, user, params = {}) described_class.new(project, user, params).public_send(async ? :async_execute : :execute) end