From 931600836b03076195f0a8df413eb546bf6353ca Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 24 Aug 2018 00:57:30 +0200 Subject: [PATCH 1/4] Changelog --- ...-timeouts-in-groupdestroyworker-due-to-sitestatistics.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/50441-high-number-of-statement-timeouts-in-groupdestroyworker-due-to-sitestatistics.yml diff --git a/changelogs/unreleased/50441-high-number-of-statement-timeouts-in-groupdestroyworker-due-to-sitestatistics.yml b/changelogs/unreleased/50441-high-number-of-statement-timeouts-in-groupdestroyworker-due-to-sitestatistics.yml new file mode 100644 index 00000000000..3e360f8d6bb --- /dev/null +++ b/changelogs/unreleased/50441-high-number-of-statement-timeouts-in-groupdestroyworker-due-to-sitestatistics.yml @@ -0,0 +1,5 @@ +--- +title: Removing a group no longer triggers hooks for project deletion twice +merge_request: 21366 +author: +type: fixed From a32a410fb9bfcd378840b61426a54fd56d2ebd33 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 23 Aug 2018 23:54:43 +0200 Subject: [PATCH 2/4] Move wiki statistics deletion to after_destroy --- app/models/project.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 8f631d7f0ed..37231df979e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -85,8 +85,8 @@ class Project < ActiveRecord::Base after_create :create_project_feature, unless: :project_feature after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) } - before_destroy ->(project) { project.project_feature.untrack_statistics_for_deletion! } - after_destroy -> { SiteStatistic.untrack(STATISTICS_ATTRIBUTE) } + before_destroy ->(project) { project.project_feature } # keep reference so we can untrack later + after_destroy :untrack_site_statistics after_create :create_ci_cd_settings, unless: :ci_cd_settings, @@ -2093,6 +2093,11 @@ class Project < ActiveRecord::Base Gitlab::PagesTransfer.new.rename_project(path_before, self.path, namespace.full_path) end + def untrack_site_statistics + SiteStatistic.untrack(STATISTICS_ATTRIBUTE) + SiteStatistic.project_feature.untrack_statistics_for_deletion! + end + def execute_rename_repository_hooks!(full_path_before) # 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. From 2e719bda0aec3970bcda06a21aa1e5eb41db323b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 24 Aug 2018 00:52:35 +0200 Subject: [PATCH 3/4] don't trigger project deletion hooks twice when removing a group --- app/models/project.rb | 5 ++--- app/services/groups/destroy_service.rb | 5 ++++- spec/services/groups/destroy_service_spec.rb | 8 ++++++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 37231df979e..e042c2e6dd6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -85,8 +85,7 @@ class Project < ActiveRecord::Base after_create :create_project_feature, unless: :project_feature after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) } - before_destroy ->(project) { project.project_feature } # keep reference so we can untrack later - after_destroy :untrack_site_statistics + before_destroy :untrack_site_statistics after_create :create_ci_cd_settings, unless: :ci_cd_settings, @@ -2095,7 +2094,7 @@ class Project < ActiveRecord::Base def untrack_site_statistics SiteStatistic.untrack(STATISTICS_ATTRIBUTE) - SiteStatistic.project_feature.untrack_statistics_for_deletion! + self.project_feature.untrack_statistics_for_deletion! end def execute_rename_repository_hooks!(full_path_before) diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 12aeba4af71..93d84bd8a9c 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -12,12 +12,15 @@ module Groups def execute 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. success = ::Projects::DestroyService.new(project, current_user).execute raise DestroyError, "Project #{project.id} can't be deleted" unless success end + # reload the relation to prevent triggering destroy hooks on the projects again + group.projects.reload + group.children.each do |group| # This needs to be synchronous since the namespace gets destroyed below DestroyService.new(group, current_user).execute diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index d80d0f5a8a8..97a88b5d697 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -35,6 +35,14 @@ describe Groups::DestroyService do it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) } 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 let!(:chat_team) { create(:chat_team, namespace: group) } From 414c54cec2e6e3b9c6e4cf003b78a637464b6d8b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sun, 26 Aug 2018 11:24:55 +0200 Subject: [PATCH 4/4] recalculate SiteStatistics --- ...80826111825_recalculate_site_statistics.rb | 27 +++++++++++++++++++ db/schema.rb | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 db/post_migrate/20180826111825_recalculate_site_statistics.rb diff --git a/db/post_migrate/20180826111825_recalculate_site_statistics.rb b/db/post_migrate/20180826111825_recalculate_site_statistics.rb new file mode 100644 index 00000000000..741035a444f --- /dev/null +++ b/db/post_migrate/20180826111825_recalculate_site_statistics.rb @@ -0,0 +1,27 @@ +# 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 diff --git a/db/schema.rb b/db/schema.rb index f5ce7df60e8..24f1fe71007 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # 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 enable_extension "plpgsql"