Merge branch 'sh-batch-dependent-destroys' into 'master'

Fix project destruction failing due to idle in transaction timeouts

Closes #44610

See merge request gitlab-org/gitlab-ce!18609

app/models/concerns/batch_destroy_dependent_associations.rb

+# Provides a way to work around Rails issue where dependent objects are all
+# loaded into memory before destroyed: https://github.com/rails/rails/issues/22510.
+#
+# This concern allows an ActiveRecord module to destroy all its dependent
+# associations in batches. The idea is borrowed from https://github.com/thisismydesign/batch_dependent_associations.
+#
+# The differences here with that gem:
+#
+# 1. We allow excluding certain associations.
+# 2. We don't need to support delete_all since we can use the EachBatch concern.
+module BatchDestroyDependentAssociations
+  extend ActiveSupport::Concern
+
+  DEPENDENT_ASSOCIATIONS_BATCH_SIZE = 1000
+
+  def dependent_associations_to_destroy
+    self.class.reflect_on_all_associations(:has_many).select { |assoc| assoc.options[:dependent] == :destroy }
+  end
+
+  def destroy_dependent_associations_in_batches(exclude: [])
+    dependent_associations_to_destroy.each do |association|
+      next if exclude.include?(association.name)
+
+      # rubocop:disable GitlabSecurity/PublicSend
+      public_send(association.name).find_each(batch_size: DEPENDENT_ASSOCIATIONS_BATCH_SIZE, &:destroy)
+    end
+  end
+end

app/models/project.rb

@@ -24,6 +24,7 @@ class Project < ActiveRecord::Base
   include ChronicDurationAttribute
   include FastDestroyAll::Helpers
   include WithUploads
+  include BatchDestroyDependentAssociations
 
   extend Gitlab::ConfigHelper
 

app/services/projects/destroy_service.rb

@@ -137,7 +137,13 @@ module Projects
 
         trash_repositories!
 
-        project.team.truncate
+        # Rails attempts to load all related records into memory before
+        # destroying: https://github.com/rails/rails/issues/22510
+        # This ensures we delete records in batches.
+        #
+        # Exclude container repositories because its before_destroy would be
+        # called multiple times, and it doesn't destroy any database records.
+        project.destroy_dependent_associations_in_batches(exclude: [:container_repositories])
         project.destroy!
       end
     end

changelogs/unreleased/sh-batch-dependent-destroys.yml

+---
+title: Fix project destruction failing due to idle in transaction timeouts
+merge_request:
+author:
+type: fixed

spec/models/concerns/batch_destroy_dependent_associations_spec.rb

+require 'spec_helper'
+
+describe BatchDestroyDependentAssociations do
+  class TestProject < ActiveRecord::Base
+    self.table_name = 'projects'
+
+    has_many :builds, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+    has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+    has_many :pages_domains
+    has_many :todos
+
+    include BatchDestroyDependentAssociations
+  end
+
+  describe '#dependent_associations_to_destroy' do
+    set(:project) { TestProject.new }
+
+    it 'returns the right associations' do
+      expect(project.dependent_associations_to_destroy.map(&:name)).to match_array([:builds])
+    end
+  end
+
+  describe '#destroy_dependent_associations_in_batches' do
+    set(:project) { create(:project) }
+    set(:build) { create(:ci_build, project: project) }
+    set(:notification_setting) { create(:notification_setting, project: project) }
+    let!(:todos) { create(:todo, project: project) }
+
+    it 'destroys multiple builds' do
+      create(:ci_build, project: project)
+
+      expect(Ci::Build.count).to eq(2)
+
+      project.destroy_dependent_associations_in_batches
+
+      expect(Ci::Build.count).to eq(0)
+    end
+
+    it 'destroys builds in batches' do
+      expect(project).to receive_message_chain(:builds, :find_each).and_yield(build)
+      expect(build).to receive(:destroy).and_call_original
+
+      project.destroy_dependent_associations_in_batches
+
+      expect(Ci::Build.count).to eq(0)
+      expect(Todo.count).to eq(1)
+      expect(User.count).to be > 0
+      expect(NotificationSetting.count).to eq(User.count)
+    end
+
+    it 'excludes associations' do
+      project.destroy_dependent_associations_in_batches(exclude: [:builds])
+
+      expect(Ci::Build.count).to eq(1)
+      expect(Todo.count).to eq(1)
+      expect(User.count).to be > 0
+      expect(NotificationSetting.count).to eq(User.count)
+    end
+  end
+end