Fix project destruction failing due to idle in transaction timeoutssh-batch-dependent-destroys

When deleting associated records, Rails loads all associations into memory
(https://github.com/rails/rails/issues/22510) before destroying them. This
can cause a surge in memory and cause destruction of objects to fail
due to idle in transaction database timeouts. This fix is inspired from
https://github.com/thisismydesign to destroy `has_many` relationships
in batches.

Closes #44610

5 files changed, 101 insertions, 1 deletions

diff --git a/app/models/concerns/batch_destroy_dependent_associations.rb b/app/models/concerns/batch_destroy_dependent_associations.rb
new file mode 100644
index 00000000000..353ee2e73d0
--- /dev/null
+++ b/app/models/concerns/batch_destroy_dependent_associations.rb
@@ -0,0 +1,28 @@
+# 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
diff --git a/app/models/project.rb b/app/models/project.rb
index 0fe9f8880b4..e275ac4dc6f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -24,6 +24,7 @@ class Project < ActiveRecord::Base
   include ChronicDurationAttribute
   include FastDestroyAll::Helpers
   include WithUploads
+  include BatchDestroyDependentAssociations
 
   extend Gitlab::ConfigHelper
 
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index adbc498d0bf..04a293a0fd6 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -124,7 +124,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
diff --git a/changelogs/unreleased/sh-batch-dependent-destroys.yml b/changelogs/unreleased/sh-batch-dependent-destroys.yml
new file mode 100644
index 00000000000..e297badc1fa
--- /dev/null
+++ b/changelogs/unreleased/sh-batch-dependent-destroys.yml
@@ -0,0 +1,5 @@
+---
+title: Fix project destruction failing due to idle in transaction timeouts
+merge_request:
+author:
+type: fixed
diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb
new file mode 100644
index 00000000000..c16b245bea8
--- /dev/null
+++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb
@@ -0,0 +1,60 @@
+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