Skip to content

Commit e440977

Browse files
Wrap evaluation of db/seeds.rb with the executor
Before rails#34953, when using the `:async` Active Job queue adapter, jobs enqueued in `db/seeds.rb`, such as Active Storage analysis jobs, would cause a hang (see rails#34939). Therefore, rails#34953 changed all jobs enqueued in `db/seeds.rb` to use the `:inline` queue adapter instead. (This behavior was later limited to only take effect when the `:async` adapter was configured, see rails#35905.) However, inline jobs in `db/seeds.rb` cleared `CurrentAttributes` values (see rails#37526). Therefore, rails#37568 changed the `:inline` adapter to wrap each job in its own thread, for isolation. However, wrapping a job in its own thread affects which database connection it uses. Thus inline jobs can no longer execute within the calling thread's database transaction, including seeing any uncommitted changes. Additionally, if the calling thread is not wrapped with the executor, the inline job thread (which is wrapped with the executor) can deadlock on the load interlock. And when testing (with `connection_pool.lock_thread = true`), the inline job thread can deadlock on one of the locks added by rails#28083. Therefore, this commit reverts the solutions of rails#34953 and rails#37568, and instead wraps evaluation of `db/seeds.rb` with the executor. This eliminates the original hang from rails#34939, which was also due to running multiple threads and not wrapping all of them with the executor. And, because nested calls to `executor.wrap` are ignored, any inline jobs in `db/seeds.rb` will not clear `CurrentAttributes` values. Alternative fix for rails#34939. Reverts rails#34953. Reverts rails#35905. Partially reverts rails#35896. Alternative fix for rails#37526. Reverts rails#37568. Fixes rails#40552. (cherry picked from commit 648da12)
1 parent 7e9102b commit e440977

File tree

6 files changed

+35
-76
lines changed

6 files changed

+35
-76
lines changed

activejob/lib/active_job/queue_adapters/inline_adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module QueueAdapters
1212
# Rails.application.config.active_job.queue_adapter = :inline
1313
class InlineAdapter
1414
def enqueue(job) #:nodoc:
15-
Thread.new { Base.execute(job.serialize) }.join
15+
Base.execute(job.serialize)
1616
end
1717

1818
def enqueue_at(*) #:nodoc:

activejob/test/integration/queuing_test.rb

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
require "jobs/logging_job"
55
require "jobs/hello_job"
66
require "jobs/provider_jid_job"
7-
require "jobs/thread_job"
87
require "active_support/core_ext/numeric/time"
98

109
class QueuingTest < ActiveSupport::TestCase
@@ -146,21 +145,4 @@ class QueuingTest < ActiveSupport::TestCase
146145
assert job_executed "#{@id}.2"
147146
assert job_executed_at("#{@id}.2") < job_executed_at("#{@id}.1")
148147
end
149-
150-
test "inline jobs run on separate threads" do
151-
skip unless adapter_is?(:inline)
152-
153-
after_job_thread = Thread.new do
154-
ThreadJob.latch.wait
155-
assert_nil Thread.current[:job_ran]
156-
assert ThreadJob.thread[:job_ran]
157-
ThreadJob.test_latch.count_down
158-
end
159-
160-
ThreadJob.perform_later
161-
162-
after_job_thread.join
163-
164-
assert_nil Thread.current[:job_ran]
165-
end
166148
end

activejob/test/jobs/thread_job.rb

Lines changed: 0 additions & 22 deletions
This file was deleted.

railties/lib/rails/engine.rb

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
require "rails/railtie"
44
require "rails/engine/railties"
5+
require "active_support/callbacks"
56
require "active_support/core_ext/module/delegation"
67
require "active_support/core_ext/object/try"
78
require "pathname"
@@ -422,6 +423,9 @@ def find(path)
422423
end
423424
end
424425

426+
include ActiveSupport::Callbacks
427+
define_callbacks :load_seed
428+
425429
delegate :middleware, :root, :paths, to: :config
426430
delegate :engine_name, :isolated?, to: :class
427431

@@ -559,13 +563,7 @@ def config
559563
# Blog::Engine.load_seed
560564
def load_seed
561565
seed_file = paths["db/seeds.rb"].existent.first
562-
return unless seed_file
563-
564-
if config.try(:active_job)&.queue_adapter == :async
565-
with_inline_jobs { load(seed_file) }
566-
else
567-
load(seed_file)
568-
end
566+
run_callbacks(:load_seed) { load(seed_file) } if seed_file
569567
end
570568

571569
initializer :load_environment_config, before: :load_environment_hook, group: :all do
@@ -637,6 +635,12 @@ def load_seed
637635
end
638636
end
639637

638+
initializer :wrap_executor_around_load_seed do |app|
639+
self.class.set_callback(:load_seed, :around) do |engine, seeds_block|
640+
app.executor.wrap(&seeds_block)
641+
end
642+
end
643+
640644
initializer :engines_blank_point do
641645
# We need this initializer so all extra initializers added in engines are
642646
# consistently executed after all the initializers above across all engines.
@@ -678,18 +682,6 @@ def load_config_initializer(initializer) # :doc:
678682
end
679683
end
680684

681-
def with_inline_jobs
682-
queue_adapter = config.active_job.queue_adapter
683-
ActiveSupport.on_load(:active_job) do
684-
self.queue_adapter = :inline
685-
end
686-
yield
687-
ensure
688-
ActiveSupport.on_load(:active_job) do
689-
self.queue_adapter = queue_adapter
690-
end
691-
end
692-
693685
def has_migrations?
694686
paths["db/migrate"].existent.any?
695687
end

railties/lib/rails/railtie.rb

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require "rails/initializable"
4+
require "active_support/descendants_tracker"
45
require "active_support/inflector"
56
require "active_support/core_ext/module/introspection"
67
require "active_support/core_ext/module/delegation"
@@ -135,6 +136,7 @@ module Rails
135136
class Railtie
136137
autoload :Configuration, "rails/railtie/configuration"
137138

139+
extend ActiveSupport::DescendantsTracker
138140
include Initializable
139141

140142
ABSTRACT_RAILTIES = %w(Rails::Railtie Rails::Engine Rails::Application)
@@ -144,13 +146,7 @@ class << self
144146
delegate :config, to: :instance
145147

146148
def subclasses
147-
@subclasses ||= []
148-
end
149-
150-
def inherited(base)
151-
unless base.abstract_railtie?
152-
subclasses << base
153-
end
149+
super.reject(&:abstract_railtie?)
154150
end
155151

156152
def rake_tasks(&blk)

railties/test/railties/engine_test.rb

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -879,29 +879,40 @@ def index
879879
assert Bukkits::Engine.config.bukkits_seeds_loaded
880880
end
881881

882-
test "jobs are ran inline while loading seeds with async adapter configured" do
882+
test "loading seed data is wrapped by the executor" do
883883
app_file "db/seeds.rb", <<-RUBY
884-
Rails.application.config.seed_queue_adapter = ActiveJob::Base.queue_adapter
884+
Rails.application.config.seeding_wrapped_by_executor = Rails.application.executor.active?
885885
RUBY
886886

887887
boot_rails
888888
Rails.application.load_seed
889889

890-
assert_instance_of ActiveJob::QueueAdapters::InlineAdapter, Rails.application.config.seed_queue_adapter
891-
assert_instance_of ActiveJob::QueueAdapters::AsyncAdapter, ActiveJob::Base.queue_adapter
890+
assert_predicate Rails.application.config, :seeding_wrapped_by_executor
892891
end
893892

894-
test "jobs are ran with original adapter while loading seeds with custom adapter configured" do
893+
test "inline jobs do not clear CurrentAttributes when loading seed data" do
895894
app_file "db/seeds.rb", <<-RUBY
896-
Rails.application.config.seed_queue_adapter = ActiveJob::Base.queue_adapter
895+
class SeedsAttributes < ActiveSupport::CurrentAttributes
896+
attribute :foo
897+
end
898+
899+
class SeedsJob < ActiveJob::Base
900+
self.queue_adapter = :inline
901+
def perform
902+
Rails.application.config.seeds_job_ran = true
903+
end
904+
end
905+
906+
SeedsAttributes.foo = 42
907+
SeedsJob.perform_later
908+
Rails.application.config.seeds_attributes_foo = SeedsAttributes.foo
897909
RUBY
898910

899911
boot_rails
900-
Rails.application.config.active_job.queue_adapter = :delayed_job
901912
Rails.application.load_seed
902913

903-
assert_instance_of ActiveJob::QueueAdapters::DelayedJobAdapter, Rails.application.config.seed_queue_adapter
904-
assert_instance_of ActiveJob::QueueAdapters::DelayedJobAdapter, ActiveJob::Base.queue_adapter
914+
assert Rails.application.config.seeds_job_ran
915+
assert_equal 42, Rails.application.config.seeds_attributes_foo
905916
end
906917

907918
test "seed data can be loaded when ActiveJob is not present" do

0 commit comments

Comments
 (0)