ActiveRecord::Relation#destroy_all perform its work in batches

💇‍♀️

Fix specs

Update activerecord/lib/active_record/relation.rb

Co-authored-by: Eugene Kenny <elkenny@gmail.com>

Update activerecord/lib/active_record/relation.rb

Co-authored-by: Viktar Basharymau <6alliapumob@gmail.com>

Use class attribute instead of mattr_accessor

Add destroy all test case around value returned

add Rails.application.config.active_record.destroy_all_in_batches too new defaults framework

Reset Active Record Relation

Document ActiveRecord::Relation#destroy_all

Add changelog entry and update docs

💇‍♀️

Update method signature and argument docs

Apply suggestions from code review

Co-authored-by: Viktar Basharymau <6alliapumob@gmail.com>
Co-authored-by: Alberto Almagro <albertoalmagro@gmail.com>

activerecord/CHANGELOG.md

@@ -1,3 +1,24 @@
+*   Relation#destroy_all perform its work in batches
+
+    Since destroy_all actually loads the entire relation and then iteratively destroys the records one by one,
+    you can blow your memory gasket very easily. So let's do the right thing by default
+    and do this work in batches of 100 by default and allow you to specify
+    the batch size like so: #destroy_all(batch_size: 100).
+
+    Apps upgrading to 7.0 will get a deprecation warning. As of Rails 7.1, destroy_all will no longer
+    return the collection of objects that were destroyed.
+
+    To transition to the new behaviour set the following in an initializer:
+
+    ```ruby
+    config.active_record.destroy_all_in_batches = true
+    ```
+
+    The option is on by default for newly generated Rails apps. Can be set in
+    an initializer to prevent differences across environments.
+
+    *Genadi Samokovarov*, *Roberto Miranda*
+
 *   Adds support for `if_not_exists` to `add_foreign_key` and `if_exists` to `remove_foreign_key`.
 
     Applications can set their migrations to ignore exceptions raised when adding a foreign key

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -137,7 +137,7 @@ module ActiveRecord
           case method
           when :destroy
             if scope.klass.primary_key
-              count = scope.destroy_all.count(&:destroyed?)
+              count = scope.each(&:destroy).count(&:destroyed?)
             else
               scope.each(&:_run_destroy_callbacks)
               count = scope.delete_all

activerecord/lib/active_record/core.rb

@@ -77,6 +77,8 @@ module ActiveRecord
 
       class_attribute :default_shard, instance_writer: false
 
+      class_attribute :destroy_all_in_batches, instance_accessor: false, default: false
+
       def self.application_record_class? # :nodoc:
         if ActiveRecord.application_record_class
           self == ActiveRecord.application_record_class

activerecord/lib/active_record/relation.rb

@@ -560,8 +560,9 @@ module ActiveRecord
     # Destroys the records by instantiating each
     # record and calling its {#destroy}[rdoc-ref:Persistence#destroy] method.
     # Each object's callbacks are executed (including <tt>:dependent</tt> association options).
-    # Returns the collection of objects that were destroyed; each will be frozen, to
-    # reflect that no changes should be made (since they can't be persisted).
+    # Returns the collection of objects that were destroyed if
+    # +config.active_record.destroy_all_in_batches+ is set to +false+. Each
+    # will be frozen, to reflect that no changes should be made (since they can't be persisted).
     #
     # Note: Instantiation, callback execution, and deletion of each
     # record can be time consuming when you're removing many records at
@@ -573,8 +574,40 @@ module ActiveRecord
     # ==== Examples
     #
     #   Person.where(age: 0..18).destroy_all
-    def destroy_all
-      records.each(&:destroy).tap { reset }
+    #
+    # If +config.active_record.destroy_all_in_batches+ is set to +true+, it will ensure
+    # to perform the record's deletion in batches
+    # and destroy_all won't longer return the collection of the deleted records
+    #
+    # ==== Options
+    # * <tt>:start</tt> - Specifies the primary key value to start from, inclusive of the value.
+    # * <tt>:finish</tt> - Specifies the primary key value to end at, inclusive of the value.
+    # * <tt>:batch_size</tt> - Specifies the size of the batch. Defaults to 1000.
+    # * <tt>:error_on_ignore</tt> - Overrides the application config to specify if an error should be raised when
+    #   an order is present in the relation.
+    # * <tt>:order</tt> - Specifies the primary key order (can be :asc or :desc). Defaults to :asc.
+    #
+    # NOTE: These arguments are honoured only if +config.active_record.destroy_all_in_batches+ is set to +true+.
+    #
+    # ==== Examples
+    #
+    #   # Let's process from record 10_000 on, in batches of 2000.
+    #   Person.destroy_all(start: 10_000, batch_size: 2000)
+    #
+    def destroy_all(start: nil, finish: nil, batch_size: 1000, error_on_ignore: nil, order: :asc)
+      if ActiveRecord::Base.destroy_all_in_batches
+        batch_options = { of: batch_size, start: start, finish: finish, error_on_ignore: error_on_ignore, order: order }
+        in_batches(**batch_options).each_record(&:destroy).tap { reset }
+      else
+        ActiveSupport::Deprecation.warn(<<~MSG.squish)
+          As of Rails 7.1, destroy_all will no longer return the collection
+          of objects that were destroyed.
+          To transition to the new behaviour set the following in an
+          initializer:
+          Rails.application.config.active_record.destroy_all_in_batches = true
+        MSG
+        records.each(&:destroy).tap { reset }
+      end
     end
 
     # Deletes the records without instantiating the records

activerecord/test/cases/helper.rb

@@ -29,6 +29,9 @@ ARTest.connect
 # Quote "type" if it's a reserved word for the current connection.
 QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name("type")
 
+# FIXME: Remove this in Rails 7.1 when it's no longer needed.
+ActiveRecord::Base.destroy_all_in_batches = true
+
 def current_adapter?(*types)
   types.any? do |type|
     ActiveRecord::ConnectionAdapters.const_defined?(type) &&

activerecord/test/cases/relation/delete_all_test.rb

@@ -12,18 +12,37 @@ class DeleteAllTest < ActiveRecord::TestCase
   def test_destroy_all
     davids = Author.where(name: "David")
 
+    assert_difference("Author.count", -1) do
+      davids.destroy_all
+    end
+  end
+
+  def test_destroy_all_no_longer_returns_a_collection
+    davids = Author.where(name: "David")
+
+    assert_nil davids.destroy_all
+  end
+
+  def test_destroy_all_deprecated_returns_collection
+    ActiveRecord::Base.destroy_all_in_batches = false
+    davids = Author.where(name: "David")
+
     # Force load
     assert_equal [authors(:david)], davids.to_a
     assert_predicate davids, :loaded?
 
-    assert_difference("Author.count", -1) do
-      destroyed = davids.destroy_all
-      assert_equal [authors(:david)], destroyed
-      assert_predicate destroyed.first, :frozen?
+    assert_deprecated do
+      assert_difference("Author.count", -1) do
+        destroyed = davids.destroy_all
+        assert_equal [authors(:david)], destroyed
+        assert_predicate destroyed.first, :frozen?
+      end
     end
 
     assert_equal [], davids.to_a
     assert_predicate davids, :loaded?
+  ensure
+    ActiveRecord::Base.destroy_all_in_batches = true
   end
 
   def test_delete_all

activerecord/test/cases/relations_test.rb

@@ -1846,8 +1846,22 @@ class RelationTest < ActiveRecord::TestCase
 
     assert_difference("Post.count", -3) { david.posts.destroy_by(body: "hello") }
 
-    destroyed = Author.destroy_by(id: david.id)
-    assert_equal [david], destroyed
+    assert_difference("Author.count", -1) do
+      Author.destroy_by(id: david.id)
+    end
+  end
+
+  def test_destroy_all_deprecated_return_value
+    ActiveRecord::Base.destroy_all_in_batches = false
+    david = authors(:david)
+
+    assert_deprecated do
+      assert_difference("Author.count", -1) do
+        assert_equal [david], Author.where(id: david.id).destroy_all
+      end
+    end
+  ensure
+    ActiveRecord::Base.destroy_all_in_batches = true
   end
 
   test "find_by with hash conditions returns the first matching record" do

guides/source/configuring.md

@@ -511,6 +511,11 @@ The schema dumper adds two additional configuration options:
   `fk_rails_` are not exported to the database schema dump.
   Defaults to `/^fk_rails_[0-9a-f]{10}$/`.
 
+* `config.active_record.destroy_all_in_batches` ensures
+  ActiveRecord::Relation#destroy_all to perform the record's deletion in batches.
+  ActiveRecord::Relation#destroy_all won't longer return the collection of the deleted
+  records after enabling this option.
+
 ### Configuring Action Controller
 
 `config.action_controller` includes a number of configuration settings:

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt

@@ -43,3 +43,6 @@
 # of the video).
 # Rails.application.config.active_storage.video_preview_arguments =
 #   "-vf 'select=eq(n\\,0)+eq(key\\,1)+gt(scene\\,0.015),loop=loop=-1:size=2,trim=start_frame=1' -frames:v 1 -f image2"
+#
+# Enforce destroy in batches when calling ActiveRecord::Relation#destroy_all
+Rails.application.config.active_record.destroy_all_in_batches = true