Commit 42c050db authored by Markus Koller's avatar Markus Koller

Merge branch '26259_destroy_webhooks_before_the_project' into 'master'

Explicitly destroy webhooks and logs before the project deletion [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!59754
parents 2f66d595 6d3c1254
...@@ -116,6 +116,7 @@ module Projects ...@@ -116,6 +116,7 @@ module Projects
log_destroy_event log_destroy_event
trash_relation_repositories! trash_relation_repositories!
trash_project_repositories! trash_project_repositories!
destroy_web_hooks! if Feature.enabled?(:destroy_webhooks_before_the_project, project, default_enabled: :yaml)
# 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
...@@ -131,6 +132,23 @@ module Projects ...@@ -131,6 +132,23 @@ module Projects
log_info("Attempting to destroy #{project.full_path} (#{project.id})") log_info("Attempting to destroy #{project.full_path} (#{project.id})")
end end
# The project can have multiple webhooks with hundreds of thousands of web_hook_logs.
# By default, they are removed with "DELETE CASCADE" option defined via foreign_key.
# But such queries can exceed the statement_timeout limit and fail to delete the project.
# (see https://gitlab.com/gitlab-org/gitlab/-/issues/26259)
#
# To prevent that we use WebHooks::DestroyService. It deletes logs in batches and
# produces smaller and faster queries to the database.
def destroy_web_hooks!
project.hooks.find_each do |web_hook|
result = ::WebHooks::DestroyService.new(current_user).sync_destroy(web_hook)
unless result[:status] == :success
raise_error(s_('DeleteProject|Failed to remove webhooks. Please try again or contact administrator.'))
end
end
end
def remove_registry_tags def remove_registry_tags
return true unless Gitlab.config.registry.enabled return true unless Gitlab.config.registry.enabled
return false unless remove_legacy_registry_tags return false unless remove_legacy_registry_tags
......
---
name: destroy_webhooks_before_the_project
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59754
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/328393
milestone: '13.12'
type: development
group: group::source code
default_enabled: false
...@@ -10531,6 +10531,9 @@ msgstr "" ...@@ -10531,6 +10531,9 @@ msgstr ""
msgid "DeleteProject|Failed to remove some tags in project container registry. Please try again or contact administrator." msgid "DeleteProject|Failed to remove some tags in project container registry. Please try again or contact administrator."
msgstr "" msgstr ""
msgid "DeleteProject|Failed to remove webhooks. Please try again or contact administrator."
msgstr ""
msgid "DeleteProject|Failed to remove wiki repository. Please try again or contact administrator." msgid "DeleteProject|Failed to remove wiki repository. Please try again or contact administrator."
msgstr "" msgstr ""
......
...@@ -418,6 +418,54 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -418,6 +418,54 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
end end
end end
context 'when project has webhooks' do
let!(:web_hook1) { create(:project_hook, project: project) }
let!(:web_hook2) { create(:project_hook, project: project) }
let!(:another_project_web_hook) { create(:project_hook) }
let!(:web_hook_log) { create(:web_hook_log, web_hook: web_hook1) }
it 'deletes webhooks and logs related to project' do
expect_next_instance_of(WebHooks::DestroyService, user) do |instance|
expect(instance).to receive(:sync_destroy).with(web_hook1).and_call_original
end
expect_next_instance_of(WebHooks::DestroyService, user) do |instance|
expect(instance).to receive(:sync_destroy).with(web_hook2).and_call_original
end
expect do
destroy_project(project, user)
end.to change(WebHook, :count).by(-2)
.and change(WebHookLog, :count).by(-1)
end
context 'when an error is raised deleting webhooks' do
before do
allow_next_instance_of(WebHooks::DestroyService) do |instance|
allow(instance).to receive(:sync_destroy).and_return(message: 'foo', status: :error)
end
end
it_behaves_like 'handles errors thrown during async destroy', "Failed to remove webhooks"
end
context 'when "destroy_webhooks_before_the_project" flag is disabled' do
before do
stub_feature_flags(destroy_webhooks_before_the_project: false)
end
it 'does not call WebHooks::DestroyService' do
expect(WebHooks::DestroyService).not_to receive(:new)
expect do
destroy_project(project, user)
end.to change(WebHook, :count).by(-2)
.and change(WebHookLog, :count).by(-1)
expect(another_project_web_hook.reload).to be
end
end
end
context 'error while destroying', :sidekiq_inline do context 'error while destroying', :sidekiq_inline do
let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:pipeline) { create(:ci_pipeline, project: project) }
let!(:builds) { create_list(:ci_build, 2, :artifacts, pipeline: pipeline) } let!(:builds) { create_list(:ci_build, 2, :artifacts, pipeline: pipeline) }
......
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