brandonhilkert / sucker_punch

Sucker Punch is a Ruby asynchronous processing library using concurrent-ruby, heavily influenced by Sidekiq and girl_friday.
MIT License
2.65k stars 114 forks source link

Memoized method values #83

Closed larryfox closed 9 years ago

larryfox commented 9 years ago

Not sure if this is an issue with sucker_punch or celluloid in general (or if it's expected behavior), but memoizing methods doesn't play well with one of them. Here's a basic example:

require 'sucker_punch'

class MyJob
  include SuckerPunch::Job

  def perform(msg)
    File.open("#{msg}.txt", 'w+') { |io| io << expensive_method(msg) }
  end

  def expensive_method(msg)
    @expensive_method ||= "Your message was: #{msg}"
  end
end

MyJob.new.perform("1")
MyJob.new.perform("2")
MyJob.new.perform("3")
MyJob.new.perform("4")

Which leaves me with the following files:

1.txt ==> "Your message was: 1"
2.txt ==> "Your message was: 2"
3.txt ==> "Your message was: 1"
4.txt ==> "Your message was: 2"

Seems to be tied to the number of workers. Easy enough to work around but a gotcha non the less.

jlecour commented 9 years ago

It appears that it's not only a problem with memoized methods, but with instance variables in general.

We've been scratching our heads with a similar problem.

jlecour commented 9 years ago

Nope ! My bad.

We've found that SuckerPunch is creating a new object (an instance of the job class) for each worker, but not each time a job is instantiated.

In @larryfox example, MyJob.new is executed 5 times, but there are only 2 workers (default value), so there are only two distinct instances of them. It's easy to show with #object_id.

Then if an instance variable is memoized (in the first job execution of each worker), it's never changed again.

brandonhilkert commented 9 years ago

Sucker Punch is made possible by Celluloid pools. In order to act like a queue the instance is assigned to the registry and recalled later when you enqueue a new job. So the behavior you describe is not surprising.

I'll make a note in the troubleshooting section of the README.

In general, I tend to write the implementation of my job in other models/services. The job, then, becomes a delegator and acts more like a class method. Going down that route might help avoid these gotchas.

hmnhf commented 8 years ago

@brandonhilkert Seems you forgot to add a note about this.