Commit b4bfb098 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch 'remove-ci-optimize-project-records-destruction-ff' into 'master'

[feature flag] Removes all ci_optimize_project_records_destruction

See merge request gitlab-org/gitlab!72600
parents 757dd41a 651259ee
...@@ -130,10 +130,7 @@ module Projects ...@@ -130,10 +130,7 @@ module Projects
destroy_events! destroy_events!
destroy_web_hooks! destroy_web_hooks!
destroy_project_bots! destroy_project_bots!
if ::Feature.enabled?(:ci_optimize_project_records_destruction, project, default_enabled: :yaml)
destroy_ci_records! destroy_ci_records!
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
......
---
name: ci_optimize_project_records_destruction
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71342
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341936
milestone: '14.4'
type: development
group: group::pipeline execution
default_enabled: false
...@@ -55,33 +55,17 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -55,33 +55,17 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
.and change { Ci::Pipeline.count }.by(-1) .and change { Ci::Pipeline.count }.by(-1)
end end
context 'with ci_optimize_project_records_destruction disabled' do
stub_feature_flags(ci_optimize_project_records_destruction: false)
it 'avoids N+1 queries' do it 'avoids N+1 queries' do
recorder = ActiveRecord::QueryRecorder.new { destroy_project(project, user, {}) } recorder = ActiveRecord::QueryRecorder.new { destroy_project(project, user, {}) }
project = create(:project, :repository, namespace: user.namespace) project = create(:project, :repository, namespace: user.namespace)
pipeline = create(:ci_pipeline, project: project) pipeline = create(:ci_pipeline, project: project)
builds = create_list(:ci_build, 3, :artifacts, pipeline: pipeline) builds = create_list(:ci_build, 3, :artifacts, pipeline: pipeline)
create(:ci_pipeline_artifact, pipeline: pipeline)
create_list(:ci_build_trace_chunk, 3, build: builds[0]) create_list(:ci_build_trace_chunk, 3, build: builds[0])
expect { destroy_project(project, project.owner, {}) }.not_to exceed_query_limit(recorder) expect { destroy_project(project, project.owner, {}) }.not_to exceed_query_limit(recorder)
end end
end
context 'with ci_optimize_project_records_destruction enabled' do
it 'avoids N+1 queries' do
recorder = ActiveRecord::QueryRecorder.new { destroy_project(project, user, {}) }
project = create(:project, :repository, namespace: user.namespace)
pipeline = create(:ci_pipeline, project: project)
builds = create_list(:ci_build, 3, :artifacts, pipeline: pipeline)
create_list(:ci_build_trace_chunk, 3, build: builds[0])
expect { destroy_project(project, project.owner, {}) }.not_to exceed_query_limit(recorder)
end
end
it_behaves_like 'deleting the project' it_behaves_like 'deleting the project'
end end
...@@ -116,11 +100,11 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -116,11 +100,11 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
destroy_project(project, user, {}) destroy_project(project, user, {})
end end
context 'with running pipelines to be aborted' 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) }
it 'executes DestroyPipelineService for project ci pipelines' do it 'bulks-fails with AbortPipelineService and then executes DestroyPipelineService for each pipelines' do
allow(::Ci::DestroyPipelineService).to receive(:new).and_return(destroy_pipeline_service) allow(::Ci::DestroyPipelineService).to receive(:new).and_return(destroy_pipeline_service)
expect(::Ci::AbortPipelinesService) expect(::Ci::AbortPipelinesService)
...@@ -128,33 +112,11 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -128,33 +112,11 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
.with(project.all_pipelines, :project_deleted) .with(project.all_pipelines, :project_deleted)
pipelines.each do |pipeline| pipelines.each do |pipeline|
expect(destroy_pipeline_service) expect(destroy_pipeline_service).to receive(:execute).with(pipeline)
.to receive(:execute)
.with(pipeline)
end end
destroy_project(project, user, {}) destroy_project(project, user, {})
end end
context 'with ci_optimize_project_records_destruction disabled' do
before do
stub_feature_flags(ci_optimize_project_records_destruction: false)
end
it 'bulk-fails project ci pipelines' do
expect(::Ci::AbortPipelinesService)
.to receive_message_chain(:new, :execute)
.with(project.all_pipelines, :project_deleted)
destroy_project(project, user, {})
end
it 'does not destroy CI records via DestroyPipelineService' do
expect(::Ci::DestroyPipelineService).not_to receive(:new)
destroy_project(project, user, {})
end
end
end end
context 'when project has remote mirrors' do context 'when project has remote mirrors' do
......
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