Commit ad985d8d authored by Nick Thomas's avatar Nick Thomas

Merge branch...

Merge branch '50441-high-number-of-statement-timeouts-in-groupdestroyworker-due-to-sitestatistics' into 'master'

Resolve "High number of statement timeouts in GroupDestroyWorker due to SiteStatistics"

Closes #50441

See merge request gitlab-org/gitlab-ce!21366
parents 270c7b6f 414c54ce
...@@ -85,8 +85,7 @@ class Project < ActiveRecord::Base ...@@ -85,8 +85,7 @@ class Project < ActiveRecord::Base
after_create :create_project_feature, unless: :project_feature after_create :create_project_feature, unless: :project_feature
after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) } after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) }
before_destroy ->(project) { project.project_feature.untrack_statistics_for_deletion! } before_destroy :untrack_site_statistics
after_destroy -> { SiteStatistic.untrack(STATISTICS_ATTRIBUTE) }
after_create :create_ci_cd_settings, after_create :create_ci_cd_settings,
unless: :ci_cd_settings, unless: :ci_cd_settings,
...@@ -2099,6 +2098,11 @@ class Project < ActiveRecord::Base ...@@ -2099,6 +2098,11 @@ class Project < ActiveRecord::Base
Gitlab::PagesTransfer.new.rename_project(path_before, self.path, namespace.full_path) Gitlab::PagesTransfer.new.rename_project(path_before, self.path, namespace.full_path)
end end
def untrack_site_statistics
SiteStatistic.untrack(STATISTICS_ATTRIBUTE)
self.project_feature.untrack_statistics_for_deletion!
end
def execute_rename_repository_hooks!(full_path_before) def execute_rename_repository_hooks!(full_path_before)
# When we import a project overwriting the original project, there # When we import a project overwriting the original project, there
# is a move operation. In that case we don't want to send the instructions. # is a move operation. In that case we don't want to send the instructions.
......
...@@ -12,12 +12,15 @@ module Groups ...@@ -12,12 +12,15 @@ module Groups
def execute def execute
group.prepare_for_destroy group.prepare_for_destroy
group.projects.each do |project| group.projects.includes(:project_feature).each do |project|
# Execute the destruction of the models immediately to ensure atomic cleanup. # Execute the destruction of the models immediately to ensure atomic cleanup.
success = ::Projects::DestroyService.new(project, current_user).execute success = ::Projects::DestroyService.new(project, current_user).execute
raise DestroyError, "Project #{project.id} can't be deleted" unless success raise DestroyError, "Project #{project.id} can't be deleted" unless success
end end
# reload the relation to prevent triggering destroy hooks on the projects again
group.projects.reload
group.children.each do |group| group.children.each do |group|
# This needs to be synchronous since the namespace gets destroyed below # This needs to be synchronous since the namespace gets destroyed below
DestroyService.new(group, current_user).execute DestroyService.new(group, current_user).execute
......
---
title: Removing a group no longer triggers hooks for project deletion twice
merge_request: 21366
author:
type: fixed
# frozen_string_literal: true
class RecalculateSiteStatistics < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
transaction do
execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql? # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967
execute("UPDATE site_statistics SET repositories_count = (SELECT COUNT(*) FROM projects)")
end
transaction do
execute('SET LOCAL statement_timeout TO 0') if Gitlab::Database.postgresql? # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48967
execute("UPDATE site_statistics SET wikis_count = (SELECT COUNT(*) FROM project_features WHERE wiki_access_level != 0)")
end
end
def down
# No downside in keeping the counter up-to-date
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20180816193530) do ActiveRecord::Schema.define(version: 20180826111825) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
......
...@@ -35,6 +35,14 @@ describe Groups::DestroyService do ...@@ -35,6 +35,14 @@ describe Groups::DestroyService do
it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) } it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) }
end end
context 'site statistics' do
it 'doesnt trigger project deletion hooks twice' do
expect_any_instance_of(Project).to receive(:untrack_site_statistics).once
destroy_group(group, user, async)
end
end
context 'mattermost team' do context 'mattermost team' do
let!(:chat_team) { create(:chat_team, namespace: group) } let!(:chat_team) { create(:chat_team, namespace: group) }
......
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