From c773ae65af0af7ebfc067f64f9a80bdb6cbfb937 Mon Sep 17 00:00:00 2001 From: Nick Holden Date: Fri, 4 Feb 2022 13:32:24 -0800 Subject: [PATCH] Add `active_record.destroy_association_async_batch_size` configuration This allows applications to specify the maximum number of records that will be destroyed in a single background job by the `dependent: :destroy_async` association option. By default, the current behavior will remain the same: when a parent record is destroyed, all dependent records will be destroyed in a single background job. If the number of dependent records is greater than this configuration, the records will be destroyed in multiple background jobs. At GitHub, we have a custom method for destroying associated records in the background that we'd like to replace with `dependent: :destroy_async`. Some associations have a large number of dependent records, and our infrastructure requires that background jobs complete quickly, so we limit the maximum number of dependent records destroyed in a single background job and enqueue additional jobs when the number of records exceeds that limit. --- activerecord/CHANGELOG.md | 12 +++++++ .../associations/has_many_association.rb | 18 ++++++----- activerecord/lib/active_record/core.rb | 10 ++++++ .../destroy_association_async_test.rb | 31 ++++++++++++++++++- guides/source/configuring.md | 4 +++ .../test/application/configuration_test.rb | 18 +++++++++++ 6 files changed, 84 insertions(+), 9 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 52e0f18f46..c6be024dc3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,15 @@ +* Add `active_record.destroy_association_async_batch_size` configuration + + This allows applications to specify the maximum number of records that will + be destroyed in a single background job by the `dependent: :destroy_async` + association option. By default, the current behavior will remain the same: + when a parent record is destroyed, all dependent records will be destroyed + in a single background job. If the number of dependent records is greater + than this configuration, the records will be destroyed in multiple + background jobs. + + *Nick Holden* + * Fix `remove_foreign_key` with `:if_exists` option when foreign key actually exists. *fatkodima* diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 70ba6ddb0f..2985d0b571 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -39,14 +39,16 @@ module ActiveRecord assoc.public_send(primary_key_column) end - enqueue_destroy_association( - owner_model_name: owner.class.to_s, - owner_id: owner.id, - association_class: reflection.klass.to_s, - association_ids: ids, - association_primary_key_column: primary_key_column, - ensuring_owner_was_method: options.fetch(:ensuring_owner_was, nil) - ) + ids.each_slice(owner.class.destroy_association_async_batch_size || ids.size) do |ids_batch| + enqueue_destroy_association( + owner_model_name: owner.class.to_s, + owner_id: owner.id, + association_class: reflection.klass.to_s, + association_ids: ids_batch, + association_primary_key_column: primary_key_column, + ensuring_owner_was_method: options.fetch(:ensuring_owner_was, nil) + ) + end end else delete_all diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 576f8db779..b04c5f18d7 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -25,6 +25,16 @@ module ActiveRecord # Specifies the job used to destroy associations in the background class_attribute :destroy_association_async_job, instance_writer: false, instance_predicate: false, default: false + ## + # :singleton-method: + # + # Specifies the maximum number of records that will be destroyed in a + # single background job by the +dependent: :destroy_async+ association + # option. When +nil+ (default), all dependent records will be destroyed + # in a single background job. If specified, the records to be destroyed + # will be split into multiple background jobs. + class_attribute :destroy_association_async_batch_size, instance_writer: false, instance_predicate: false, default: nil + ## # Contains the database configuration - as is typically stored in config/database.yml - # as an ActiveRecord::DatabaseConfigurations object. diff --git a/activerecord/test/activejob/destroy_association_async_test.rb b/activerecord/test/activejob/destroy_association_async_test.rb index 527db85527..51b5819546 100644 --- a/activerecord/test/activejob/destroy_association_async_test.rb +++ b/activerecord/test/activejob/destroy_association_async_test.rb @@ -31,7 +31,9 @@ class DestroyAssociationAsyncTest < ActiveRecord::TestCase book.tags << [tag, tag2] book.save! - book.destroy + assert_enqueued_jobs 1, only: ActiveRecord::DestroyAssociationAsyncJob do + book.destroy + end assert_difference -> { Tag.count }, -2 do perform_enqueued_jobs only: ActiveRecord::DestroyAssociationAsyncJob @@ -84,6 +86,33 @@ class DestroyAssociationAsyncTest < ActiveRecord::TestCase DlKeyedJoin.delete_all end + test "enqueues multiple jobs if count of dependent records to destroy is greater than batch size" do + ActiveRecord::Base.destroy_association_async_batch_size = 1 + + tag = Tag.create!(name: "Der be treasure") + tag2 = Tag.create!(name: "Der be rum") + book = BookDestroyAsync.create! + book.tags << [tag, tag2] + book.save! + + job_1_args = ->(job_args) { job_args.first[:association_ids] == [tag.id] } + job_2_args = ->(job_args) { job_args.first[:association_ids] == [tag2.id] } + + assert_enqueued_with(job: ActiveRecord::DestroyAssociationAsyncJob, args: job_1_args) do + assert_enqueued_with(job: ActiveRecord::DestroyAssociationAsyncJob, args: job_2_args) do + book.destroy + end + end + + assert_difference -> { Tag.count }, -2 do + perform_enqueued_jobs only: ActiveRecord::DestroyAssociationAsyncJob + end + ensure + Tag.delete_all + BookDestroyAsync.delete_all + ActiveRecord::Base.destroy_association_async_batch_size = nil + end + test "belongs to" do essay = EssayDestroyAsync.create!(name: "Der be treasure") book = BookDestroyAsync.create!(name: "Arr, matey!") diff --git a/guides/source/configuring.md b/guides/source/configuring.md index d066cc114a..c6926261b5 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -902,6 +902,10 @@ The default value depends on the `config.load_defaults` target version: Allows specifying the job that will be used to destroy the associated records in background. It defaults to `ActiveRecord::DestroyAssociationAsyncJob`. +#### `config.active_record.destroy_association_async_batch_size` + +Allows specifying the maximum number of records that will be destroyed in a background job by the `dependent: :destroy_async` association option. All else equal, a lower batch size will enqueue more, shorter-running background jobs, while a higher batch size will enqueue fewer, longer-running background jobs. This option defaults to `nil`, which will cause all dependent records for a given association to be destroyed in the same background job. + #### `config.active_record.queues.destroy` Allows specifying the Active Job queue to use for destroy jobs. When this option is `nil`, purge jobs are sent to the default Active Job queue (see `config.active_job.default_queue_name`). It defaults to `nil`. diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index af151d09ba..a1c2805962 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2460,6 +2460,24 @@ module ApplicationTests assert_equal DummyDestroyAssociationAsyncJob, ActiveRecord::Base.destroy_association_async_job end + test "destroy association async batch size is nil by default" do + app "development" + + assert_nil ActiveRecord::Base.destroy_association_async_batch_size + end + + test "destroy association async batch size can be set in configs" do + app_file "config/environments/development.rb", <<-RUBY + Rails.application.configure do + config.active_record.destroy_association_async_batch_size = 100 + end + RUBY + + app "development" + + assert_equal 100, ActiveRecord::Base.destroy_association_async_batch_size + end + test "ActionView::Helpers::FormTagHelper.default_enforce_utf8 is false by default" do app "development" assert_equal false, ActionView::Helpers::FormTagHelper.default_enforce_utf8