Commit b545abbf authored by Kerri Miller's avatar Kerri Miller

Extract MergeRequestDiffCommit deletions

Projects will have at least one merge_request_diff_commit for every commit
contained in every MR, which deleting via `project.destroy!` and
cascading deletes may exceed statement timeouts, causing failures.
(see https://gitlab.com/gitlab-org/gitlab/-/issues/346166)

Changelog: fixed
parent b8cd7ab7
...@@ -140,6 +140,10 @@ module Projects ...@@ -140,6 +140,10 @@ module Projects
destroy_project_bots! destroy_project_bots!
destroy_ci_records! destroy_ci_records!
if ::Feature.enabled?(:extract_mr_diff_commit_deletions, default_enabled: :yaml)
destroy_mr_diff_commits!
end
# Rails attempts to load all related records into memory before # Rails attempts to load all related records into memory before
# destroying: https://github.com/rails/rails/issues/22510 # destroying: https://github.com/rails/rails/issues/22510
# This ensures we delete records in batches. # This ensures we delete records in batches.
...@@ -154,6 +158,33 @@ module Projects ...@@ -154,6 +158,33 @@ module Projects
log_info("Attempting to destroy #{project.full_path} (#{project.id})") log_info("Attempting to destroy #{project.full_path} (#{project.id})")
end end
# Projects will have at least one merge_request_diff_commit for every commit
# contained in every MR, which deleting via `project.destroy!` and
# cascading deletes may exceed statement timeouts, causing failures.
# (see https://gitlab.com/gitlab-org/gitlab/-/issues/346166)
#
# rubocop: disable CodeReuse/ActiveRecord
def destroy_mr_diff_commits!
mr_batch_size = 100
delete_batch_size = 1000
project.merge_requests.each_batch(column: :iid, of: mr_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))
.limit(delete_batch_size)
deleted_rows = MergeRequestDiffCommit
.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
end
# rubocop: enable CodeReuse/ActiveRecord
def destroy_ci_records! def destroy_ci_records!
project.all_pipelines.find_each(batch_size: BATCH_SIZE) do |pipeline| # rubocop: disable CodeReuse/ActiveRecord project.all_pipelines.find_each(batch_size: BATCH_SIZE) do |pipeline| # rubocop: disable CodeReuse/ActiveRecord
# Destroy artifacts, then builds, then pipelines # Destroy artifacts, then builds, then pipelines
......
---
name: extract_mr_diff_commit_deletions
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75963
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/347073
milestone: '14.6'
type: development
group: group::code review
default_enabled: false
...@@ -97,6 +97,20 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -97,6 +97,20 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
end end
end end
shared_examples_for "deleting a project with merge requests" do
let!(:merge_request) { create(:merge_request, source_project: project) }
it "deletes merge request and related records" do
merge_request_diffs = merge_request.merge_request_diffs
expect(merge_request_diffs.size).to eq(1)
mrdc_count = MergeRequestDiffCommit.where(merge_request_diff_id: merge_request_diffs.first.id).count
expect { destroy_project(project, user, {}) }
.to change { MergeRequestDiffCommit.count }.by(-mrdc_count)
end
end
it_behaves_like 'deleting the project' it_behaves_like 'deleting the project'
it 'invalidates personal_project_count cache' do it 'invalidates personal_project_count cache' do
...@@ -105,6 +119,25 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -105,6 +119,25 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
destroy_project(project, user, {}) destroy_project(project, user, {})
end end
context "extract_mr_diff_commit_deletions feature flag" do
context "with flag enabled" do
before do
stub_feature_flags(extract_mr_diff_commit_deletions: true)
allow(project).to receive(:destroy!).and_return(true)
end
it_behaves_like "deleting a project with merge requests"
end
context "with flag disabled" do
before do
stub_feature_flags(extract_mr_diff_commit_deletions: false)
end
it_behaves_like "deleting a project with merge requests"
end
end
context 'with running pipelines' do context 'with running pipelines' do
let!(:pipelines) { create_list(:ci_pipeline, 3, :running, project: project) } let!(:pipelines) { create_list(:ci_pipeline, 3, :running, project: project) }
let(:destroy_pipeline_service) { double('DestroyPipelineService', execute: nil) } let(:destroy_pipeline_service) { double('DestroyPipelineService', execute: nil) }
......
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