Skip to content

Commit

Permalink
Don't clear non-SuckerPunch actors from registry
Browse files Browse the repository at this point in the history
* Currently `SuckerPunch::Railtie` clears all actors from the Celluloid
  actor registry during initialization. This is problematic because an
  application using SuckerPunch may have additional Celluloid actors.
* In this commit I propose prefixing the names of SuckerPunch
  actors such that we can ensure the Celluloid registry is cleared only
  of SuckerPunch actors on application initialization, per @nullstyle's
  suggestion here: #99
* Update `SuckerPunch::Queue#name` to add a prefix to queue names
* Introduce `SuckerPunch.clear_queues`
* Introduce `SuckerPunch::Queue.clear_all`
* Update `SuckerPunch::Railtie` to use `SuckerPunch.clear_actors`
  • Loading branch information
Laila Winner committed May 12, 2015
1 parent 90b0094 commit 74a7ce6
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 6 deletions.
4 changes: 4 additions & 0 deletions lib/sucker_punch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ def self.logger=(logger)
def self.exception_handler(&block)
Celluloid.exception_handler(&block)
end

def self.clear_queues
SuckerPunch::Queue.clear_all
end
end

require 'sucker_punch/railtie' if defined?(::Rails)
15 changes: 14 additions & 1 deletion lib/sucker_punch/queue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class Queue
attr_accessor :pool

DEFAULT_OPTIONS = { workers: 2 }
PREFIX = "sucker_punch"
class MaxWorkersExceeded < StandardError; end
class NotEnoughWorkers < StandardError; end

Expand All @@ -14,6 +15,17 @@ def self.find(klass)
Celluloid::Actor[queue.name]
end

def self.clear_all
Celluloid::Actor.all.each do |actor|
registered_name = actor.registered_name.to_s
matches = registered_name.match(PREFIX).to_a

if matches.any?
Celluloid::Actor.delete(registered_name)
end
end
end

def initialize(klass)
@klass = klass
@pool = nil
Expand All @@ -39,7 +51,8 @@ def registered?
end

def name
klass.to_s.underscore.to_sym
klass_name = klass.to_s.underscore
"#{PREFIX}_#{klass_name}".to_sym
end

private
Expand Down
2 changes: 1 addition & 1 deletion lib/sucker_punch/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Railtie < ::Rails::Railtie
end

config.to_prepare do
Celluloid::Actor.clear_registry
SuckerPunch.clear_queues
end
end
end
40 changes: 36 additions & 4 deletions spec/sucker_punch/queue_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,40 @@ def perform(name)
Celluloid::Actor.clear_registry
end

describe "#find" do
describe ".find" do
it "returns the Celluloid Actor from the registry" do
SuckerPunch::Queue.new(FakeJob).register
queue = SuckerPunch::Queue.find(FakeJob)
queue.class == Celluloid::PoolManager
end
end

describe ".clear_all" do
it "removes SuckerPunch actors from Celluloid registry" do
sucker_punch_actor_name = "#{SuckerPunch::Queue::PREFIX}_fake_job".to_sym
Celluloid::Actor[sucker_punch_actor_name] = FakeJob.new

SuckerPunch.clear_queues

expect(Celluloid::Actor[sucker_punch_actor_name]).to be_nil
end

it "does not remove non-SuckerPunch actors from Celluloid registry" do
class ::OtherJob
include ::Celluloid
def self.pool(options); end
def perform; end
end
actor_name = :other_job
job = OtherJob.new
Celluloid::Actor[actor_name] = job

SuckerPunch.clear_queues

expect(Celluloid::Actor[actor_name]).to eq job
end
end

describe "#register" do
let(:job) { FakeJob }
let(:queue) { SuckerPunch::Queue.new(job) }
Expand All @@ -33,13 +59,19 @@ def perform(name)
end

it "registers the pool with Celluloid" do
expected_pool_name = "#{SuckerPunch::Queue::PREFIX}_fake_job".to_sym

pool = queue.register
expect(Celluloid::Actor[:fake_job]).to eq(pool)

expect(Celluloid::Actor[expected_pool_name]).to eq(pool)
end

it "registers the pool with Celluloid and 3 workers" do
pool = queue.register(3)
expect(Celluloid::Actor[:fake_job].size).to eq(3)
expected_pool_name = "#{SuckerPunch::Queue::PREFIX}_fake_job"

queue.register(3)

expect(Celluloid::Actor[expected_pool_name].size).to eq(3)
end

context "when too many workers are specified" do
Expand Down
10 changes: 10 additions & 0 deletions spec/sucker_punch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,14 @@
SuckerPunch.logger = nil
end
end

describe '.clear_queues' do
it "clears SuckerPunch queues" do
allow(SuckerPunch::Queue).to receive(:clear_all)

SuckerPunch.clear_queues

expect(SuckerPunch::Queue).to have_received(:clear_all)
end
end
end

0 comments on commit 74a7ce6

Please sign in to comment.