Skip to content
Permalink
Browse files
Initial queue implementation
  • Loading branch information
wycats committed Apr 27, 2012
1 parent 770c809 commit adff4a7
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 3 deletions.
@@ -22,6 +22,7 @@
module Rails
autoload :Info, 'rails/info'
autoload :InfoController, 'rails/info_controller'
autoload :Queueing, 'rails/queueing'

class << self
def application
@@ -37,6 +38,25 @@ def configuration
application.config
end

# Rails.queue is the application's queue. You can push a job onto
# the queue by:
#
# Rails.queue.push job

This comment has been minimized.

Copy link
@cscotta

cscotta Apr 27, 2012

The task queue is implemented as a singleton, and there's only one per app? For many applications which may communicate with a variety of backing work queues, this approach would not be feasible.

This comment has been minimized.

Copy link
@fizx

fizx Apr 27, 2012

Contributor

+1 I'd prefer Rails.queue.default and Rails.queue.high_priority and ... (a la OStruct)

This comment has been minimized.

Copy link
@tomjakubowski

tomjakubowski Apr 27, 2012

+1, prefer fizx's suggestion

This comment has been minimized.

Copy link
@rsanheim

rsanheim Apr 28, 2012

Contributor

No singletons please. Does not work for many many apps.

This comment has been minimized.

Copy link
@tarcieri

tarcieri Apr 28, 2012

Contributor

I think Rails.queue[:name].push job is probably a more practical API. Using method_missing like that precludes certain queue names because they're already valid method names, unless you really want to go the BasicObject route.

Rails.queue could delegate to the default queue, but also respond to #[] to choose alternative queue names.

#
# A job is an object that responds to +run+. Queue consumers will

This comment has been minimized.

Copy link
@mfilej

mfilej Apr 27, 2012

Contributor

Just thinking - how about call instead of run to allow lambdas?

This comment has been minimized.

Copy link
@tarcieri

tarcieri Apr 28, 2012

Contributor

Lambdas don't marshal and are impractical for queues which run outside the same Ruby VM

This comment has been minimized.

Copy link
@romanbsd

romanbsd May 14, 2012

Contributor

Maybe "perform" is a more sensible name for the method it should respond to. This will allow to use the delayed_job workers verbatim.

This comment has been minimized.

Copy link
@RobertLowe

RobertLowe May 14, 2012

@romanbsd

resque uses self.perform
delayed_job uses perform
lambda use call

After some thought I think it's fair to have devs port code to the standardize API (whatever is finalized) as the change is small. Most involve shuffling methods around that's all. That said call would make the most sense for lambda compatibility.

This comment has been minimized.

Copy link
@guilleiguaran

guilleiguaran May 14, 2012

Member

@RobertLowe the election of run was probably intentional to avoid the use of lambdas

This comment has been minimized.

Copy link
@romanbsd

romanbsd May 14, 2012

Contributor

@RobertLowe that was my first thought (#call), because I can clearly see the benefit of calling this method #call, as I believe that there will be enough apps that will just utilize the ThreadedConsumer, if a more robust queueing is not required. I currently have a very similar home-made approach for performing some OOB http requests, which enqueues lambdas. The very low traffic profile makes a fully-fledged queueing solution a sure overkill in this case.

This comment has been minimized.

Copy link
@RobertLowe

RobertLowe May 14, 2012

@guilleiguaran hmm, but how is that of benefit?

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik May 14, 2012

Member

Ruby classes that are 'executable' use #call. That's my vote, personally.

This comment has been minimized.

Copy link
@guilleiguaran

guilleiguaran May 14, 2012

Member

@RobertLowe See the comment of @tarcieri in this thread "Lambdas don't marshal and are impractical for queues which run outside the same Ruby VM"

This comment has been minimized.

Copy link
@RobertLowe

RobertLowe May 14, 2012

@guilleiguaran Valid point, quite right. @pauldowman & I were just chatting about this issue, we thought that you should have class method to define what is async. Like in delayed_job's handle_asynchronously, because for instance in resque you end up writing proxies to existing classes which just feels messy.

class Post < ActiveRecord::Base
   queuable :my_queue_job
end

This comment has been minimized.

Copy link
@steveklabnik

steveklabnik May 14, 2012

Member

@guilleiguaran nobody's saying marshall lambdas. #call is a general convention.

This comment has been minimized.

Copy link
@wycats

wycats May 14, 2012

Author Member

Using #call would imply that Ruby "callables" are, in general, usable. And in fact, they would seem to work in development mode, and then fail hard in production. The choice of run was to clarify that Procs are not usable here, and make it very hard to do by accident.

This comment has been minimized.

Copy link
@romanbsd

romanbsd May 14, 2012

Contributor

@wycats they'll work fine in production mode as far as the original Threaded implementation is used, and I believe that it will be widely used. It'll replace the home-brew solutions and girl_friday gem for the most of us.

This comment has been minimized.

Copy link
@wycats

wycats May 14, 2012

Author Member

The more they work, the worse the problem is. The protocol requires marshallability, and this will hide protocol failures.

# pop jobs off of the queue and invoke the queue's +run+ method.

This comment has been minimized.

Copy link
@balinterdi

balinterdi Apr 28, 2012

Contributor

... and invoke the job's (or its) run method, if I'm not mistaken.

#
# Note that depending on your queue implementation, jobs may not
# be executed in the same process as they were created in, and
# are never executed in the same thread as they were created in.
#
# If necessary, a queue implementation may need to serialize your
# job for distribution to another process. The documentation of
# your queue will specify the requirements for that serialization.

This comment has been minimized.

Copy link
@mboeh

mboeh Apr 28, 2012

Most queuing systems require some type of serialization, other than the default one implemented here. I think leaving the details and requirements of serialization entirely to the queuing system will result in chaos to the point of making this API difficult to rely on in any third-party code (gems, etc.).

Ruby already has (several) built-in serialization protocols. Requiring _dump/_load at minimum does not strike me as a particularly onerous requirement. Individual queue systems can implement more sophisticated serialization for objects if desired.

This comment has been minimized.

Copy link
@tarcieri

tarcieri Apr 28, 2012

Contributor

Though it may not be expressed in the present documentation, from my conversations with @wycats I believe the expectation is for the data to be Marshallable. This was the reasoning behind the decision to use #run instead of #call (see the concerns noted above) as Procs can't be serialized, so #run was chosen to preclude people from trying to use them.

def queue
application.queue
end

def initialize!
application.initialize!
end
@@ -66,7 +66,7 @@ def inherited(base)
end
end

attr_accessor :assets, :sandbox
attr_accessor :assets, :sandbox, :queue
alias_method :sandbox?, :sandbox
attr_reader :reloaders

@@ -199,6 +199,10 @@ def config #:nodoc:
@config ||= Application::Configuration.new(find_root_with_flag("config.ru", Dir.pwd))
end

def queue #:nodoc:
@queue ||= config.queue.new

This comment has been minimized.

Copy link
@benlangfeld

benlangfeld May 3, 2012

This is kinda evil. Surely the config system should hold an instance rather than the class name, so that I can instantiate however I like.

This comment has been minimized.

Copy link
@comboy

comboy May 5, 2012

I very much agree.

This comment has been minimized.

Copy link
@tarcieri

tarcieri May 5, 2012

Contributor

It does. See the attr_accessor here

This comment has been minimized.

Copy link
@benlangfeld

benlangfeld May 5, 2012

This should match: adff4a7#L4R40

end

def to_app
self
end
@@ -11,7 +11,7 @@ class Configuration < ::Rails::Engine::Configuration
:force_ssl, :helpers_paths, :logger, :log_formatter, :log_tags, :preload_frameworks,
:railties_order, :relative_url_root, :secret_token,
:serve_static_assets, :ssl_options, :static_cache_control, :session_options,
:time_zone, :reload_classes_only_on_change, :use_schema_cache_dump
:time_zone, :reload_classes_only_on_change, :use_schema_cache_dump, :queue

attr_writer :log_level
attr_reader :encoding
@@ -43,6 +43,7 @@ def initialize(*)
@autoflush_log = true
@log_formatter = ActiveSupport::Logger::SimpleFormatter.new
@use_schema_cache_dump = true
@queue = Queue

@assets = ActiveSupport::OrderedOptions.new
@assets.enabled = false
@@ -93,6 +93,13 @@ module Finisher
ActiveSupport::Dependencies.unhook!
end
end

initializer :activate_queue_consumer do |app|
if config.queue == Queue
consumer = Rails::Queueing::ThreadedConsumer.start(app.queue)

This comment has been minimized.

Copy link
@jrochkind

jrochkind Apr 27, 2012

Contributor

where does one find the ThreadedConsumer implementation in this source snapshot? Thanks!

This comment has been minimized.

Copy link
@KieranP

This comment has been minimized.

Copy link
@jrochkind

jrochkind Apr 27, 2012

Contributor

@KieranP Thanks! Now I can't find where Queue, as in @queue = Queue is defined, or the single argument to ThreadedConsumer constructor. Not even sure what the full namespaced name of that class is. Any hints?

update: Aha, this is just a ruby stdlib Queue? Cool. http://www.ruby-doc.org/stdlib-1.9.3/libdoc/thread/rdoc/Queue.html

at_exit { consumer.shutdown }
end
end
end
end
end
@@ -35,4 +35,7 @@
# Expands the lines which load the assets.
config.assets.debug = true
<%- end -%>

# In development, use an in-memory queue for queueing
config.queue = Queue
end
@@ -76,4 +76,8 @@

# Use default logging formatter so that PID and timestamp are not suppressed
config.log_formatter = ::Logger::Formatter.new

# Default the production mode queue to an in-memory queue. You will probably
# want to replace this with an out-of-process queueing solution
config.queue = Queue
end
@@ -33,4 +33,7 @@

# Print deprecation notices to the stderr.
config.active_support.deprecation = :stderr

# Use the testing queue
config.queue = Rails::Queueing::TestQueue
end
@@ -8,7 +8,7 @@
# Rails booted up.
require 'fileutils'

require 'rubygems'
require 'bundler/setup'
require 'minitest/autorun'
require 'active_support/test_case'

112 comments on commit adff4a7

@wfarr
Copy link
Contributor

@wfarr wfarr commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 — having a standardized API for this stuff is a great move.

@dimitrisdovinos
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will this be different to delayed_job?

@alindeman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation is small and simple 👍

@kevinzen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back when I was a J2EE developer, I loved how JMS integrated with MQ-Series and other queueing systems. And now I use resque all the time -- but have used rabbitmq and others.

So I'm all +1 for @josevalim 's description above. Put a simple, light, common API in the framework that you can hook into whichever queue you want to use.

This paves the way for rails to penetrate deeper into big enterprise apps.

I'd like to hear the beginnings of ideas on how to make this transactional.

@toamitkumar
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh I love the fact to have a standard interface, though I don't feel comfortable of using queueing system for AM.

Not everyone sends bulk emails and queueing mail delivery makes sense for those cases. For one of cases (send an email of user creation) I feel uncomfortable. Moreover, queueing is terrible on windows.

@kevinzen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DimiDimi I'm upgrading an app now to move from delay_job to resque. With this api in place, the migration should be declarative -- simply specifying queue parameters somewhere -- rather than requiring me to write code to adapt to the new queue.

@lorennorman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@seebq
Copy link

@seebq seebq commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

I am teaching a big enterprise software company today Ruby and Ruby on Rails, and I just said: "there is no 'queueing' mechanism built-in to Rails, but there will be one day." I support this.

@PanosJee
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @micho

@brandoncordell
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh @josevalim I think it's a fantastic improvement. Some people are going to troll or disagree with every action made in a framework as popular as Rails. A lot of the people like this think frameworks like Rails should be built for them and their needs/concerns.

Three cheers for forward progress. 👍

@bobbytables
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't like the feature, then don't use it. If you don't know what it does, learn about it. This is a great feature especially for larger projects that may need to separate things into a more modular approach. Stop complaining.

@JakubOboza
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reminds me about @tenderlove talk about concurrency at railsberry :)

I like this idea, more queue enignes will be easier to use.

@apisandipas
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standardizing a queuing API from within the framework is an excellent idea. Don't use it if you don't want do, but consider how moving forward with a standard API with allow for effortlessly switching out of third-party queuing gems in the future. 10/10 would commit again.

@jwreagor
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails.cache... Rails.queue... nuff said. +1

@matzipan
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convention over configuration, so yeah, this fits in really good. Queuing is a really nice asynchronous design pattern.

EDIT: Not to mention the fact that it can really help scaling things up.

@boblmartens
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice move.

@fbjork
Copy link

@fbjork fbjork commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent addition. Totally makes sense. Can't think of any larger project I've worked on in the past few years that doesn't have queueing. Standardizing this can only be positive IMO.

@soulcutter
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I give the addition of a queueing interface a hearty +1. I'm surprised it wasn't already a part of rails, it seems so obvious in hindsight.

@micho
Copy link

@micho micho commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing some examples of what our app would use this for:

  • Async emails. There isn't a "Rails way" to send async mails.
  • Cache expiration and regeneration.
  • Nested deletion of dependents records.
  • Posting to 3rd party services (Google, etc).
  • Collecting stats.

@plukevdh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👎 Do not want. What if web workers matures and I wanted to use them instead of a server-side queue/consumer system? Then i'm still burdened with Rails queue system...

@jc00ke
Copy link

@jc00ke jc00ke commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that you decided to use the verb run when the most popular queuing libs use perform. What was your motivation for changing the verb?

Also, while a general API sounds like a good idea, considering how easy the "API" is for libraries like girl_friday I don't quite see the need for something baked into Rails that isn't as time-tested.

I do hope this will get us all more comfortable with threads.

@josepjaume
Copy link
Contributor

@josepjaume josepjaume commented on adff4a7 Apr 27, 2012 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh
Copy link
Member

@dhh dhh commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plukevdh What if we AI matures and we will no longer need to write software ourselves? Then we'd still be burdened with Rails!

We design for the world as it is today.

@hosh
Copy link

@hosh hosh commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, so if it is a way for something like Resque to hook in, then it is not so bad. I guess this reminds me a bit of the Merb #run_later code.

@plukevdh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh well put. i feel though we should design for the world as we want it to be. if i were to need something like this, i'd rather see it as a gem (which we have an abundance of) or a disable-able option, rather than tied directly to the Rails object. just my opinion.

@eldavido
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh absolutely agree on this, best to standardize the interface and then allow various strategies to plug themselves in to drain the queue. Forcing anything that uses a queue to be implementation-specific (e.g. Resque, DJ, ...) is needlessly tight coupling.

@lukeredpath
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty astounded by the number of people who have misunderstood what this commit does and those who cannot see the benefits that would result from this.

This isn't a built-in queuing system. It's an API, an abstraction, with a convenient basic in-memory implementation and easy testing built in, on which mature queueing systems like Resque can build upon and act as adapters.

This means queueing becomes consistent across Rails apps, infrastructure decisions about which queueing system you want to use can be deferred as they can be swapped out more easily and Rails can use the API internally to make certain operations (@micho pointed out some good ones) potentially asynchronous without worrying about the underlying queueing mechanism.

Regardless of your chosen queueing implementation, it's arguably good practice to abstract it behind a generic interface rather than coupling various points of your codebase to a particular queueing library so it can be easily swapped out (the same applies to any kind of third party integration). Now Rails ships with one, and its one less piece of code you need to write.

@mrrooijen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems interesting. What are the main benefits from using this in-front of a worker library such as DJ, Resque, Qu, Sidekiq, etc? Can you also use Rails.queue stand-alone without any 3rd party worker libraries? Or is it purely an interface/adapter that standardizes all these various worker libraries?

If it's for standardizing an API, won't that limit the worker libraries features? What happens if worker library A has other ways of enqueuing jobs, for example like Delayed Job: object.delay.a_method. I assume this wouldn't be available in the Rails.queue interface?

I guess the lack of information is just making it a bit vague to understand what the main purpose of this tool is. But I'm interested in knowing how one would use something like this. If all it does is allowing Resque / DJ / etc to hook in to Rails.queue.push job, what happens if you were doing for example object.delay(run_at: 1.hour.from_now).my_method? Do such features just disappear?

Edit: I guess @lukeredpath partially answered my question. Still would like to know about worker library-specific features like DJ's run_at and delay. What about process vs thread-based scaling? How do you further configure such things and various queues.

@lukeredpath
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meskyanichi it seems clear to me that this abstraction is serving the 80/20 rule. For most people, pushing stuff on to a queue and popping it off again is sufficient.

If you need the more advanced queuing features of your library of choice, then you are going to have to either use those directly, or wrap them up in your own abstraction. That doesn't make this built-in abstraction any less useful for the majority of people.

@mrrooijen
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukeredpath yeah that's what it looks like from the Rails.queue public API side. But, I guess worker libraries will always still have the freedom to allow for initializers to add additional configuration. Actually if you take (vanilla) Resque for example, I believe you also can't specify additional options to the enqueue method (correct me if I'm wrong). You just pass in the processor class and the arguments. The queues would be defined in the process classes itself.

Plus, it's likely that if for example Delayed Job would hook in to Rails.queue it will still include the delay method for convenience. For most things I like to write processor classes, but for smaller, less frequent jobs like sending ActionMailer emails I like invoking delay rather than creating a whole new class to handle the email sending. Only thing I dislike about that approach is the whole YAML serialization that happens which can become quite slow and CPU intensive with larger objects.

Probably the best and most "efficient/light-weight" way of going about push to a message queue is Rails.queue.push and pushing data that doesn't require heavy serialization like simple strings/ints. So I guess in a way Rails enforces good practice in a sense.

@sdball
Copy link

@sdball sdball commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails is a collection of opinions for the correct way to construct an application. I don't see it as outside the scope of Rails at all to have an opinion on how to implement background processes. Convention (an opinion in Rails) over configuration (installing a gem).

@bryckbost
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meskyanichi We'll be taking a close look at this, but I think that's more or less the plan going forward for DJ.

@DCarper
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm far more in favor of abstract APIs than I am... ohh... requiring a js runtime... ;)

@railsfactory
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@Veejay
Copy link
Contributor

@Veejay Veejay commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the dedication and hard work guys, you're making our lives easier.

@tysonmote
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 This is kickin' rad.

What @Veejay said.

@jonaphin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic addition.

@scottwhite
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great direction on keeping rails relevant.

@intjonathan
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, this is long overdue.

@kirillzubovsky
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh great first comment. having queues as part of rails will make it significantly easier to start for beginners. thank you and @wycats

@fesplugas
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW: Where are tests for that? 602000b

@toamitkumar
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great discussion like the excitement everyone has. Is everyone using queues for AM's ?

@taelor
Copy link

@taelor taelor commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. API's for the win.

I hope we start seeing this trend more and more, rails providing the interfaces, and devs dropping in the implementations based on whatever requirements they have. Can't install Redis in your stack? use delayed job. need more control? roll your own. wanna experiment with jRuby and Torqebox, go for it.

But the good thing is, none of your main code has to change, the only thing you had to change is the back end implementation.

@taelor
Copy link

@taelor taelor commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to think of some other possible interfaces that work in this similar manner (Rails.cache, Rails.queue). What about something like Rails.serializer?

Rails.serializer.json(@post) or Rails.serializer.xml(@post) or something along those lines? That way if you want to use builder, rabl, or ActiveModel::Serializers, you can.

What are some other common non-application specific things we can think of to build interfaces for?

Also, this actually kind of "thinks about the -kids- noobs" in a way. I think it's easier to jump from project seeing Rails.queue.push everywhere, rather than struggle with each individual implementation (at first glance).

@kristofer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want this. I end up using celery everytime...

@idontusegithub
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 @josevalim good points.

@mboeh
Copy link

@mboeh mboeh commented on adff4a7 Apr 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good move, but the specification for a job is going to eventually need to include some minimum expectations for serialization. Obviously, the purpose for a generic API like these is to let engines and such interact with the background worker queue. But the current specification permits this:

o = Object.new
def o.run() "Run!" end
Rails.queue.push o

which is unlikely to work on any implementation other than an in-process one.

Something as simple as requiring the job object to be Marshal-safe should suffice. This doesn't mean the queuing system has to use Marshal to serialize the job, but that should be available as a last-ditch option where more sophisticated approaches are not possible.

@RobertLowe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josepjaume I agree, I'd love to see this in Rails for standardizing an API, I'd love to see a rails-queue gem even more.

On a side note, I think the same should happen for ActiveResource not including by default.

We should prolly get some sort of ActionQueue & ActionQueue::Base classes as well.

@tarcieri
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobertLowe the advantage of such an API being a standard part of Rails, rather than a gem, is that it will ensure more widespread adoption, which is exactly what is needed to hammer out what details are missing from the implementation and get everyone on board and using it. The entire spirit of Rails is to set conventions everyone can use, rather than having a bunch of competing, disparate, and incompatible solutions.

The status quo in the Ruby world is that several people are locked into Resque because it presents its own API, and for many they may have grown to have needs beyond what Resque can provide, but are now locked in due to legacy.

By pulling this API into Rails itself, it will become the de facto standard, and will hopefully become a solution to the Resque lock-in we see in the Ruby community today.

@dlee
Copy link
Contributor

@dlee dlee commented on adff4a7 Apr 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh @josevalim I wonder if it would be better for the Queue interface to be standardized at a lower level, like how the HTTP server interface is standardized via Rack API.

The situation is actually very similar to how the HTTP server landscape was before Rack came about. Before Rack, individual HTTP servers had to be directly implemented for Rails. Rack came along and standardized the interface between Ruby servers and Ruby web application frameworks (including many outside of Rails).

In the same way, it would benefit the entire Ruby community to have a non-Rails-exclusive common API for Queues.

Besides standardizing the API, Rack provided another huge plus: middlewares. I'm not sure if there would be an analogous advantage provided by standardizing the Queue API.

@RobertLowe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josevalim I think an issue that needs to be addressed is persistence, will we have to lock/flag our own records?

Would it make sense if the interface was instance methods? Similar to ActionMailer

user_email_job(user).queue

Just a thought...

@jlebrech
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this also do scheduling?

If it could monitor loads then prioritise queues based on that would be pretty awesome.

@homakov
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlebrech +1 I wonder too how to tie queue + scheduling(crontab based for example).

But whenever gem is suffice so far... anyway scheduling is very common for "Do most full-size Rails applications, think Basecamp or Github"(@dhh) so yea - they Do need scheduling out of box too.

@oriolgual
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm +1000 with @dlee comment

@unclebilly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm 100% behind an abstraction API for communicating with background processing... we independently came up with something like this after moving from BackgrounDRb to Resque and suffering the pain of NOT having a layer of indirection.

Here's a couple of things I thought about this:

  • Above, it was stated that this patch is about 100 LOC - but @dlee points out (and others agree) that it would be nice to have an API that multiple applications can share, something akin to Rack. I find it unlikely that the queue-related code, were it to make it into Rails, would be 100 LOC a few years from now. It would probably be better if it were it's own thing from the get-go, even if it is so small.
  • The default implementation is in no way a sane default. Understandably, the default implementation of Queue is very naive. But it goes too far - it should never do anything with the job! It would be trivial to accidentally hook this up in such a way that it brings your site down. It would be far easy to lose queued work. There are of course a whole host of issues with it. I understand that it is merely an example to show how the API works; still, I think that the default implementation should either do less (as in just stubbing stuff) or there should not be a default implementation at all. Remember, Rails ships with an awesome ORM as the "sane default" for persistence. Why would you set people up with that expectation and then shoot them down with this? Better not to even suggest it is functional.

@tarcieri
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unclebilly I'm uncertain why you think the default queue implementation could "bring the whole site down"

Also, the default persistence engine Rails uses is SQLite. This is a great choice for onboarding new developers and local development, but obviously a terrible choice for production.

People seem to have no trouble picking a more appropriate database when they deploy their app. Why would they have trouble picking a more appropriate queue?

@unclebilly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tarcieri sorry if that came out sounding brash. As far as the "bringing down a site," it's easy to imagine a scenario where a developer, using the provided Queue, sets up a longish-running background job, which ends up being triggered by a user event on the website. All it would take would be someone to click furiously on something to effectively take a couple of webservers down.

As far as the ORM analogy - you are talking about SQLite, but I was talking about ActiveRecord. Rails developers made ActiveRecord, and it rocks. It is the default persistence framework, but you are free to swap it out with, say, ActiveResource or even DataMapper or something else. The idea is that the default is something you would actually use in production.

@tarcieri
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unclebilly queues are asynchronous. Even with a long-running job, a user "furiously clicking on the site" would need to exhaust the system memory by filling up the Queue before they could take a web server down. I just put 10 million messages into a queue without issues.

You are talking about ActiveRecord, sure. ActiveRecord provides a high-level abstraction API, much like this queue interface. Like ActiveRecord you can swap out the default adapter for whatever you want. Since most Rails apps talk to SQL databases, shipping Rails with ActiveRecord by default is a Good Idea™. Likewise, most Rails apps use queues, so shipping Rails with this queue API is also a Good Idea™.

However, you were conflating that with the default implementation being based on Ruby's Queue. So your analogy is wrong, sorry. You are conflating two different issues. The default ActiveRecord SQLite adapter, much like the default Queue adapter, is not well-suited for production use. However, it's great for both onboarding new developers and general development use, because it's simple, easy-to-use, and gets out of your way during development, then in production you can make the switch to whatever best suits your needs (or make the same switch in development as well if you are so inclined)

This has not been an issue with ActiveRecord, and I do not forsee it being an issue with the queue API.

@Systho
Copy link

@Systho Systho commented on adff4a7 Jun 25, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the whole point is about making a new API (which is awesome !) , then why not making the API part of Rails + some tests easily usable by implementors and then offer a default implementation as a Railtie ?.

I think the ORM adapter analogy is really great and I'm looking forward to see a "( delayed_job | resque | sidekiq | anything )-queue-adapter" gem.

Maybe choosing one of those queing system as default backend could be a good idea (Rails team didn't build Sqlite3, they provided the adapter ). That would allow the default implementation to be strong while documenting how to make an adapter.

PS : Sorry for my english, I'm not a native speaker :)

@edraut
Copy link

@edraut edraut commented on adff4a7 Sep 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an implicit requirement here that all jobs must be written in job-specific classes. I've got a queuing system that allows me to do this:

MyQueue.enqueue(MyClass,:instance_method,{payload: 'parameter'})

Now I don't have to clutter up my code with worker classes. The real business logic is encapsulated in some class, and that class may have multiple methods that could be run both synchronously or asynchronously. My logic might fit best into an activerecord model, and that model might have multiple methods that need to be run asynchronously. Why do I want to add a new class for every single type of job? I suspect there is some fear that we won't know which methods are run synchronously and which are run asynchronously. However, forcing all asynchronous code to go through a worker class just shifts the problem: if the worker class calls a method in a non-worker class, whoever reads that non-worker class still won't know the method is (sometimes) being called asynchronously. So what's the real reason for adding this layer of abstraction?

I suspect we're just thinking in the box. Resque uses worker classes, Sidekiq followed Resque, and so on, so it must be the right way. Please reconsider, the box we are thinking in forces us to abstract unnecessarily. Free us up to encapsulate and reuse code the way each of our individual designs require, while still providing a contract that all queuing systems can use. The contract I proposed above can still be used for Resque, just name your worker class and use the :perform instance method. Or if your background system uses 'run', by all means, name your worker class and insert :run. Just don't force us to proliferate worker classes for every kind of background task we want to do. It makes background work feel heavy, and we should be pushing more work off into the background, not less.

nb. I intended the contract above to marshall, so the class constant would be stringified, and the payload would be turned into JSON. All fully marshallable, and indeed portable to another language if you so choose. Heck, I'd even agree to making the class and instance names strings if that makes it clearer that it must be marshallable.

Please sign in to comment.