With Unicorn, production mode, and unicorn's preload_app, work isn't being completed

[scottkf]

I saw the older issue, but this again crops up where work isn't being completed if preload_app is true.

It does work when put into after_fork, however.

[bradphelan]

I can verify the problem I had to add my queue definitions to the after_fork block

after_fork do |server, worker|
  # Replace with MongoDB or whatever
  if defined?(ActiveRecord::Base)
    ActiveRecord::Base.establish_connection
    Rails.logger.info('Connected to ActiveRecord')
  end

  # If you are using Redis but not Resque, change this
  if defined?(Resque)
    Resque.redis = ENV['REDIS_URI']
    Rails.logger.info('Connected to Redis')
  end

  # Girl Friday Config
  ::COMMENT_QUEUE = GirlFriday::WorkQueue.new(:comment_queue, :size => 3) do |info|
    CommentMailer.mail_subscribers! info[:id]
  end

  ::DEVISE_QUEUE = GirlFriday::WorkQueue.new(:devise_queue, :size => 3) do |info|
    DeviseBackgroundMailer.complete_action(info).deliver
  end

  ::INVITE_QUEUE = GirlFriday::WorkQueue.new(:invite_queue, :size => 3) do |info|
    InviteMailer.invite_email(info[:id]).deliver
  end
end

[bradphelan]

I've got a better way to do this now. I use the Rails singleton model to lazily instantiate the queues. Now I don't need to pollute my unicorn config file and my RSpec tests running without unicorn still have the queues instantiated. The above code translates to.

----------------------
app/models/comment_queue.rb
----------------------
  1 class CommentQueue < GirlFriday::WorkQueue
  2   include Singleton
  3  
  4   def initialize
  5     super(:comment_queue, :size => 3) do |info|
  6       CommentMailer.mail_subscribers! info[:id]
  7     end
  8   end
  9  
 10   def self.push *args
 11     instance.push *args
 12   end
 13  
 14 end

repeated for each queue. This pattern could be folded into girl friday's internals with a bit of metaprogramming

[bradphelan]

Even nicer

class LazyWorkQueue < GirlFriday::WorkQueue

  include Singleton

  def self.define *options, &block
    Class.new LazyWorkQueue do
      define_method :initialize do
        super *options do |info|
          block.call
        end
      end
    end
  end

  def self.push *args
    instance.push *args
  end

  def self.status
    instance.status
  end

end

and I can do

InviteQueue = LazyWorkQueue.define :invite_queue, :size => 3 do |info|
  InviteMailer.invite_email(info[:id]).deliver
end

which is pretty much the same as the original code except that the queue is lazily instantiated which works with unicorn and rspec.

[mperham]

@bradphelan Nice!

[tcassanego]

@bradphelan Thanks for those solutions! I ended up using the first as it seemed quite nice to have an app model representing the queue(s).

@mperham Would it make sense to drop a small blurb in the wiki docs pointing people here if they're using unicorn? I ended up adding the preload_app to my unicorn config since that seems to be the recommended approach with unicorn on heroku. It was tough to eventually find this since I didn't realize preload_app was the problem until much later.

Thanks!

[mperham]

@tcassanego Sounds great. Why don't you add it to the wiki yourself? It's publicly editable.

[tcassanego]

@mperham Ah! Didn't realize. Will do!

[jrochkind]

I'd be careful with using a global singleton to lazily create queues and thread-safety. If two threads try to use the queues at 'just' the right time, they may end up both trying to 'initialize' the queues, with potentially odd results, depending on exactly how your code works (the different versions brad supplies may even differ in whether they are subject to this flaw, I can't tell for sure from a skim).

May not end up being a problem in GIL rubies.

But may be safest to just use the after_fork method (or passenger equivalent) and not have to worry about it.

[bradphelan]

@jrochkind You bring up a legit concern but Ruby 1.9.2's Singleton mixin is threadsafe :)

http://ruby-doc.org/stdlib-1.9.2/libdoc/singleton/rdoc/Singleton.html

See the source code for the init method.

[jrochkind]

Cool, the important thing there is that you're creating the queue in
the singleton's constructor, that's what makes singleton's
threadsafe-ness apply to queue creation here, if you were creating it
lazily in some other method you'd have to take care of synchronization
yourself.

It also means that there's a little bit of added overhead for the
mutex every time any thread asks for the singleton object. Probably
not a big deal.

I've just learned in multi-threaded programming, the simpler you keep
things and the fewer mutexes the better -- I think I'd still be using
the on_fork methods, but the singleton one should work, I agree!

[bradphelan]

@jrochkind You should look at the singleton code closely. The only time mutexes are used are if the instance needs creating. Otherwise the instance is just returned with no mutex overhead.

 def klass.instance
    return @singleton__instance__ if @singleton__instance__
    @singleton__mutex__.synchronize {
      return @singleton__instance__ if @singleton__instance__
      @singleton__instance__ = new()
    }
    @singleton__instance__
  end

The only reason for on_fork is to clean up IO handles that have been duplicated across the fork call. There is no reason for girl_friday to create those IO handles before they are actually needed. on_fork callback in unicorn is a bit of a hack to fix things that probably should be handled in a cleaner hidden way from the library user.

[jrochkind]

Aha, thanks, awesome. As far as cleaning up IO handles -- girl_friday
itself may create/use such IO handles that need to be cleaned up? So
for use with Passenger, someone should take a look at what's being
done for unicorn and try to do similar in passenger?

[gluemonkey]

@bradphelan Dude, brilliant and a real lifesaver! Thanks!

I did have to make a slight modification to get that info object across in my implementation.

class LazyWorkQueue < GirlFriday::WorkQueue

  include Singleton

  def self.define *options, &block
    Class.new LazyWorkQueue do
      define_method :initialize do
        super *options do |info|
          block.call(info)
        end
      end
    end
  end

  def self.push *args
    instance.push *args
  end

  def self.status
    instance.status
  end

end

[fallanic]

I don't know if there's a more elegant solution but @gluemonkey 's patch worked for me (and huge thanks to @bradphelan)

@mperham shall we copy this piece of code in the wiki?

[mperham]

That lazy implementation looks nice and elegant. I like the use of Singleton to delay the instance creation. Maybe we should pull it into girl_friday as the new WorkQueue?