diff options
| -rw-r--r-- | app/models/project.rb | 8 | ||||
| -rw-r--r-- | app/services/groups/destroy_service.rb | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/50441-high-number-of-statement-timeouts-in-groupdestroyworker-due-to-sitestatistics.yml | 5 | ||||
| -rw-r--r-- | db/post_migrate/20180826111825_recalculate_site_statistics.rb | 27 | ||||
| -rw-r--r-- | db/schema.rb | 2 | ||||
| -rw-r--r-- | spec/services/groups/destroy_service_spec.rb | 8 |
6 files changed, 51 insertions, 4 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 178d70757a1..67593c9b2fe 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.untrack_statistics_for_deletion! } - after_destroy -> { SiteStatistic.untrack(STATISTICS_ATTRIBUTE) } + before_destroy :untrack_site_statistics after_create :create_ci_cd_settings, unless: :ci_cd_settings, @@ -2099,6 +2098,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) + self.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. 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/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 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 380d4e49ddf..cb8f90efded 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" 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) } |