Add new inline-like built-in adapter to process it in a thread

[rosenfeld]

For development purposes, specially when you're working on improving your actions performances, it's useful to see how much time your action takes to process, considering your jobs are not running inline.

For such cases, it would be useful to have some "threaded" adapter that would work just like enqueue_at(job, Time.now) from the "inline" built-in adapter. Having this other adapter as built-in would be ideal. This would avoid the need to run Sidekiq, for instance, in the development environment.

I'm not sure if there's some mailing list for this project, so I'm creating an issue for the feature request. Please let me know if there are better places to do so.

Congrats for the initiative with this project by the way :)

[rosenfeld]

Just to illustrate the idea:

require_relative 'inline_adapter'
module ActiveJob
  module QueueAdapters
    class ThreadedAdapter < InlineAdapter
      class << self
        def enqueue(job, *args)
          enqueue_at job, Time.now, *args
        end
      end
    end
  end
end

[seuros]

You will be able to use activejob-stats (WIP) to benchmark your actions.

[rosenfeld]

I think that implementation would be to benchmark the job execution, not the action, right? Anyway, I don't want my application to run slower in my development mode if it can run faster by running the background jobs in the background rather than inline...

[seuros]

You should use the inline function of your adapter to make it run inline in dev mode.

[cristianbica]

Maybe we can extract the jobs consumer functionality from here: https://github.com/rails/rails/tree/jobs Currently, if you enqueue_in/at many jobs it will open a new thread for each of the job and a user might hit a OS limit (I'm not sure there's such limit though ...). We also should put a notice in the readme that if a process is restarted when using the inline adapter some jobs enqueued in the future will be lost

Cristian Bica

On Fri, May 30, 2014 at 3:50 PM, Rodrigo Rosenfeld Rosas < notifications@github.com> wrote:

I think that implementation would be to benchmark the job execution, not the action, right? Anyway, I don't want my application to run slower in my development mode if it can run faster by running the background jobs in the background rather than inline...

— Reply to this email directly or view it on GitHub https://github.com/rails/activejob/issues/76#issuecomment-44646829.

[cristianbica]

@seuros what's he saying is of he wants to test the show action from the products controller, which runs some jobs, he will have to subtract from the action total time the time spent on the jobs (and it will also increase the dev time ... waiting for the job)

[rosenfeld]

I think a warning that the jobs may be lost would be ok for the inline adapter. But this could be better handled anyway, by registering an at_exit hook that will wait for all jobs to finish executing.

I just don't think this is relevant to this issue as this affects the inline adapter as it is...

[seuros]

The inline adapter should be used only for test mode. If you use it in production and somehow your application restart , everything will be lost.

[rosenfeld]

No one is talking about using it in production, but rather in development mode.

[cristianbica]

@rosenfeld registering an at_exit hook that will wait for all jobs ... how about when scheduling a job 3 days from now? That won't work...

[rosenfeld]

Yes, of course the inline adapter is not meant to be robust. The at_exit hook would only care about the commonest case of immediate execution and ignore any jobs scheduled to the future. Maybe it could print a warning "N scheduled jobs have been canceled. If this behavior is not desired you should opt for another adapter that would care to persist them.". And add this warning in the documentation of the project as well.

[dhh]

I could buy this with the proper caveats. Including a rails.logger.warn if Rails.env.production? when this threaded is selected.

[rosenfeld]

We could even raise on production mode, what do you think?

[rosenfeld]

I don't think it worths supporting the use of such simple adapter on production.

[dhh]

I think a warning would be good enough. If you want to run with scissors, feel fee, we should just tell you it’s dangerous.

On Jun 2, 2014, at 6:46 PM, Rodrigo Rosenfeld Rosas notifications@github.com wrote:

We could even raise on production mode, what do you think?

— Reply to this email directly or view it on GitHub.

[rosenfeld]

I'm just worried about how do you know if the application is in production mode? If you simply check for Rails.env.production? then if someone uses staging or certification then it won't work.

[dhh]

This would go alongside the general warning at the top of this adapter in the docs. I think that’s plenty.

On Jun 2, 2014, at 6:47 PM, Rodrigo Rosenfeld Rosas notifications@github.com wrote:

I'm just worried about how do you know if the application is in production mode? If you simply check for Rails.env.production? then if someone uses staging or certification then it won't work.

— Reply to this email directly or view it on GitHub.

[rosenfeld]

Okay, I can work on that. But most probably just after Friday as I have to prepare an article for PacktPub.com on Chef and Capistrano until Thursday...

[dhh]

All good!

On Jun 2, 2014, at 6:49 PM, Rodrigo Rosenfeld Rosas notifications@github.com wrote:

Okay, I can work on that. But most probably just after Friday as I have to prepare an article for PacktPub.com on Chef and Capistrano until Thursday...

— Reply to this email directly or view it on GitHub.

[rosenfeld]

Just to be sure. Should I check if Rails is defined before Rails.logger.warn, or should I simply assume it is?

[rosenfeld]

I'm asking because railties is not a dependency of this gem: https://github.com/rails/activejob/blob/master/activejob.gemspec#L19

[seuros]

@rosenfeld , you can continue this if you want : https://github.com/seuros/activejob/tree/threaded The adapter is working , but the test are falling now.

[seuros]

Should I check if Rails is defined before Rails.logger.warn, or should I simply assume it is?

You should use ActiveJob.logger.warn

[rosenfeld]

Great, thanks!

[rosenfeld]

I just cloned and run all tests and they all passed. Were you trying to say that it's missing tests?

[rosenfeld]

ah, sorry, wrong repo.

[seuros]

If you delay the assertion just 200ms, they all pass.

[rosenfeld]

I'm closing this as I understand the maintainers have decided for not shipping any adapter that is not production-ready. If you change your mind simply reopen it and let me know and I'll work in a PR for it.