Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start async adapters after_initialize instead of once Active Job and Active Record are loaded and Rails initialized? #1297

Merged
merged 1 commit into from
Mar 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/good_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
require_relative "good_job/cron_manager"
require_relative "good_job/current_thread"
require_relative "good_job/daemon"
require_relative "good_job/dependencies"
require_relative "good_job/job_performer"
require_relative "good_job/job_performer/metrics"
require_relative "good_job/log_subscriber"
Expand All @@ -46,7 +45,6 @@
#
# +GoodJob+ is the top-level namespace and exposes configuration attributes.
module GoodJob
include GoodJob::Dependencies
include GoodJob::ThreadStatus

# Default, null, blank value placeholder.
Expand Down Expand Up @@ -114,6 +112,11 @@ module GoodJob
# @return [GoodJob::Capsule, nil]
mattr_accessor :capsule, default: GoodJob::Capsule.new(configuration: configuration)

mattr_accessor :_async_ready, default: false
def self._async_ready?
_async_ready
end

# Called with exception when a GoodJob thread raises an exception
# @param exception [Exception] Exception that was raised
# @return [void]
Expand Down
2 changes: 1 addition & 1 deletion lib/good_job/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def initialize(execution_mode: nil, _capsule: GoodJob.capsule) # rubocop:disable
GoodJob::Configuration.validate_execution_mode(@_execution_mode_override) if @_execution_mode_override
@capsule = _capsule

start_async if GoodJob.async_ready?
start_async if GoodJob._async_ready?
self.class.instances << self
end

Expand Down
29 changes: 0 additions & 29 deletions lib/good_job/dependencies.rb

This file was deleted.

25 changes: 10 additions & 15 deletions lib/good_job/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,17 @@ class Engine < ::Rails::Engine
end

initializer "good_job.start_async" do
# This hooks into the hookable places during Rails boot, which is unfortunately not Rails.application.initialized?
# If an Adapter is initialized during boot, we want to want to start async executors once the framework dependencies have loaded.
# When exactly that happens is out of our control because gems or application code may touch things earlier than expected.
# For example, as of Rails 6.1, if an ActiveRecord model is touched during boot, that triggers ActiveRecord to load,
# which touches DestroyAssociationAsyncJob, which loads ActiveJob, which may initialize a GoodJob::Adapter, all of which
# happens _before_ ActiveRecord finishes loading. GoodJob will deadlock if an async executor is started in the middle of
# ActiveRecord loading.
config.after_initialize do
ActiveSupport.on_load(:active_record) do
ActiveSupport.on_load(:active_job) do
GoodJob._framework_ready = true
GoodJob._start_async_adapters
end
GoodJob._start_async_adapters
end
GoodJob._start_async_adapters
GoodJob._async_ready = true

# Ensure Active Record and Active Job are fully loaded
ActiveRecord::Base # rubocop:disable Lint/Void
ActiveJob::Base.queue_adapter

GoodJob::Adapter.instances
.select(&:execute_async?)
.reject(&:async_started?)
.each(&:start_async)
end
end
end
Expand Down