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

Wrap Scheduler task execution with Rails reloader instead of executor to avoid database connection changing during code reload #389

Merged
merged 1 commit into from
Sep 30, 2021

Conversation

bensheldon
Copy link
Owner

@bensheldon bensheldon commented Sep 30, 2021

Connects to #388.

Related to #95, and hopefully this PR does not produce deadlocks.

…tor` to avoid database connection changing during code reload
@jjb
Copy link

jjb commented Jan 19, 2022

@bensheldon

I was recently reading https://guides.rubyonrails.org/threading_and_code_execution.html#reloader

and came across "Rails automatically wraps web requests and Active Job workers"

what do you know about how the need for this interacts with that alleged behavior?

I wonder if that statement only applies to the shipped async adapter and whoever wrote the docs wasn't precise?

@bensheldon
Copy link
Owner Author

bensheldon commented Jan 19, 2022

@jjb great question! I think the documentation is written correctly, but GoodJob is a special case because GoodJob uses ActiveRecord to fetch-and-lock the serialized job data before it invokes ActiveJob. GoodJob has to wrap its code with the Rails Executor in order for GoodJob to work correctly, and then when ActiveJob is invoked, ActiveJob's usage of Rails Executor is a no-op. E.g. in psuedo code:

Thread.new do
  Rails.application.executor.wrap do
    GoodJob::Execution.fetch_and_lock_job do |job_params|
      ActiveJob.execute(job_params) do |job|
        Rails.application.executor.wrap do # noop
          job.perform
        end
      end
    end
  end
end

I think the Rails docs are assuming that the Thread is being invoked directly with the serialized job params primitives e.g.

Thread.new(job_params) do |thr_job_params|
  ActiveJob.execute(thr_job_params) do |job|
    Rails.application.executor.wrap do # noop
      job.perform
    end
  end
end

@jjb
Copy link

jjb commented Jan 19, 2022

ahh gotcha, interesting - thanks for the explanation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants