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

[Feature Request?] Config Option to Inline Child-Jobs in Worker Processes #1052

Closed
marvinscharle opened this issue Aug 28, 2023 · 12 comments · Fixed by #1056
Closed

[Feature Request?] Config Option to Inline Child-Jobs in Worker Processes #1052

marvinscharle opened this issue Aug 28, 2023 · 12 comments · Fixed by #1056

Comments

@marvinscharle
Copy link

marvinscharle commented Aug 28, 2023

Hi @bensheldon! First of all – thank you for the great work.

We're using GoodJob to schedule all our async/delayed background processes. However, we also have a set of very heavy jobs that currently spawn a lot of (>100k) child jobs. While heavy jobs are executed on a special queue, the child jobs are enqueued on another queue, thus delaying other time-critical jobs.

Since the child jobs and the regular (time-critical) jobs use the same ActiveJob classes as the regular jobs, my question is:

Can GoodJob be configured so that child jobs spawned by the workers will be executed automatically inline? We already tried it by using this in production.rb, however it does not seem to work:

config.good_job.execution_mode = ENV['RUN_JOBS_INLINE'] ? :inline : :external

If there is already a way to configure it and I was too stupid to read the manual, I'd appreciate a hint how to do it.

Otherwise, I'd suggest this as a feature.

Cheers & thank you in advance!

@bdewater
Copy link

Sounds like this could be solved by setting the queue for these child jobs instead of using the default time-critical one?

@bensheldon
Copy link
Owner

Thanks for opening this issue. I think it makes sense conceptually that GoodJob should allow this but it doesn't currently:

mode = if GoodJob::CLI.within_exe?
:external
else
options[:execution_mode] ||
rails_config[:execution_mode] ||
env['GOOD_JOB_EXECUTION_MODE']
end

But why I think it makes sense is because "Execution Mode" is mainly a function of the Adapter at enqueue: when a job is enqueued, should it be simply put on the queue (:external), executed in a background thread in the current process (:async), or run in the foreground immediately (:inline).

When running the CLI, I guess that all of those should hold true for jobs that are enqueued as part of executing other jobs. I imagine that even :async could imply process-fixation.

I guess the question I have is: would this imply a breaking change? The situation I'm imagining is that someone has global ENV set for like :inline in the web process and they are also running a background GoodJob CLI process. That seems pretty rare though.

In the meantime though, a workaround would be to manually assign the adapter:

MyJob.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline)
# or do it everywhere
config.active_job.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) if ENV['RUN_JOBS_INLINE']

@marvinscharle
Copy link
Author

Sounds like this could be solved by setting the queue for these child jobs instead of using the default time-critical one?

Unfortunately, this does not work for us since most of the child jobs are enqueued during after_save/after_save_commit hooks.

In the meantime though, a workaround would be to manually assign the adapter:

Thank you much! I'm gonna test this on our staging system and give you feedback.

I guess the question I have is: would this imply a breaking change? The situation I'm imagining is that someone has global ENV set for like :inline in the web process and they are also running a background GoodJob CLI process. That seems pretty rare though.

For me, this behavior was not clear. I'd assume that the setting of execution_mode would overwrite any default set by GoodJob.


Yesterday we also stumbled upon another interesting behavior: We use Puma as webserver, depending on the evironment also in clustered mode.

We discovered that GoodJob automatically runs a worker on all threads.

Is there a way to prevent GoodJob from executing any jobs on the web servers (jobs should only be run on the workers)? As a workaround, we set the ENV GOOD_JOB_QUEUES to the non-existing queue none, but this seems more like a "hacky" approach:

SCR-20230829-d6b

Is there an official way to do this?

@bensheldon
Copy link
Owner

Is there a way to prevent GoodJob from executing any jobs on the web servers (jobs should only be run on the workers)?

GoodJob's default execution mode in the Development Environment is :async aka, execute jobs on the webserver. The production environment default is :external, which should not execute jobs on the webserver

Can you confirm you're seeing this in production, and the execution mode has not been configured to :async?

@marvinscharle
Copy link
Author

Yes, I can confirm that this is happening in production. How can I support you to reproduce this?

@bensheldon
Copy link
Owner

Do you have production console access? If so, could you run GoodJob.configuration.execution_mode and tell me the output?

Also, do you have this in your config/puma.rb?

on_worker_boot do
  GoodJob.restart
end

Looking into it, I think this will unfortunately start GoodJob's executor regardless of the execution mode. If that's what you have, the workaround would be:

on_worker_boot do
  GoodJob.restart if GoodJob.configuration.execution_mode == :async
end

...but I should also make that work better if that is the problem.

@bensheldon
Copy link
Owner

Oops, didn't mean to close that 😰

@marvinscharle
Copy link
Author

Do you have production console access? If so, could you run GoodJob.configuration.execution_mode and tell me the output?

Yes, it returns :external
grafik


on_worker_boot is also present in puma.rb. I'm gonna deploy the workaround to our staging environment to see if the processes are no longer started.

@marvinscharle
Copy link
Author

After deployment of the workaround to the staging environment, I can confirm that the worker processes are no longer started via Puma.

@bensheldon
Copy link
Owner

@marvinscharle fhew, glad we could figure out a workaround. I'll take a look at the design for GoodJob#restart and think about how to make it less surprising.

@marvinscharle
Copy link
Author

Thank you much!

@bensheldon
Copy link
Owner

I just released both changes mentioned here 🎉 : https://github.com/bensheldon/good_job/releases/tag/v3.18.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

3 participants