Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unlock queue on dequeue #3

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
3 changes: 1 addition & 2 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
source :rubygems

gem "resque"
gem "resque", :git => 'git://github.com/humancopy/resque.git'

group :development do
gem "turn"
gem "rake"
Copy link
Owner

Choose a reason for hiding this comment

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

Why?

Copy link
Author

Choose a reason for hiding this comment

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

Couldn't run tests with this gem. Was getting this error message:

~/.rvm/rubies/ruby-1.9.2-p180/lib/ruby/1.9.1/test/unit/assertions.rb:5:in module:Test': Unit is not a module (TypeError)`

The tests run without it and the rake file is testing if the command exists, so i figured it should be optional. :)

Copy link
Owner

Choose a reason for hiding this comment

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

Fair enough!

end
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,20 @@ UpdateNetworkGraph is queued at a time, regardless of the
repo_id. Normally a job is locked using a combination of its
class name and arguments.

It is also possible to define locks which will get released
BEFORE performing a job by overriding the lock_running? class
method in your subclass. This is useful in cases where you need
to get a job queued even if another job on same queue is already
running, e.g.

class UpdateNetworkGraph
extend Resque::Plugins::Lock

# Do not lock a running job
def self.lock_running?
false
end
end


[rq]: http://github.com/defunkt/resque
41 changes: 38 additions & 3 deletions lib/resque/plugins/lock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,62 @@ module Plugins
# UpdateNetworkGraph is running at a time, regardless of the
# repo_id. Normally a job is locked using a combination of its
# class name and arguments.
#
# It is also possible to define locks which will get released
# BEFORE performing a job by overriding the lock_running? class
# method in your subclass. This is useful in cases where you need
# to get a job queued even if another job on same queue is already
# running, e.g.
#
# class UpdateNetworkGraph
# extend Resque::Plugins::Lock
#
# # Do not lock a running job
# def self.lock_running?
# false
# end
# end
module Lock
# Override in your job to control the lock key. It is
# passed the same arguments as `perform`, that is, your job's
# payload.
def lock(*args)
"lock:#{name}-#{args.to_s}"
"#{name}-#{args.to_s}"
end

def namespaced_lock(*args)
"lock:#{lock(*args)}"

Choose a reason for hiding this comment

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

Why is namespaced_lock defined with the same behaviour lock used to have, but only use lock in namespaced_lock

end

def before_enqueue_lock(*args)
Resque.redis.setnx(lock(*args), true)
Resque.redis.setnx(namespaced_lock(*args), true)
end

def before_dequeue_lock(*args)
Resque.redis.del(namespaced_lock(*args))
end

def lock_running?
true
end

def around_perform_lock(*args)
before_dequeue_lock(*args) unless lock_running?
begin
yield
ensure
# Always clear the lock when we're done, even if there is an
# error.
Resque.redis.del(lock(*args))
before_dequeue_lock(*args) if lock_running?
end
end

def self.all_locks
Resque.redis.keys('lock:*')

Choose a reason for hiding this comment

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

From the redis documentation for keys:

Warning: consider KEYS as a command that should only be used in production environments with extreme care. It may ruin performance when it is executed against large databases. This command is intended for debugging and special operations, such as changing your keyspace layout. Don't use KEYS in your regular application code. If you're looking for a way to find keys in a subset of your keyspace, consider using sets.

This is also unrelated to the purpose of the pull request. A pull request should focus on fixing one thing to make it easier to get merged.

Choose a reason for hiding this comment

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

resque-lock should really be using a set instead of separate keys to make it easy to get a list of locks and check if locks aren't being cleaned up. I have a commit to use redis sets, but I will wait for #2 and #6 to be merged before opening a pull request to avoid conflicts.

end
def self.clear_all_locks
all_locks.collect { |x| Resque.redis.del(x) }.count
end
end
end
end
Expand Down
25 changes: 23 additions & 2 deletions test/lock_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def self.perform

def setup
Resque.redis.del('queue:lock_test')
Resque.redis.del(Job.lock)
Resque.redis.del("lock:#{Job.lock}")
Resque.redis.del("lock:#{Job.lock(1)}")
end

def test_lint
Expand All @@ -37,4 +38,24 @@ def test_lock

assert_equal 1, Resque.redis.llen('queue:lock_test')
end
end

def test_lock_with_args
3.times { Resque.enqueue(Job, 1) }

assert_equal 1, Resque.redis.llen('queue:lock_test')
end

def test_unlock
Resque.enqueue(Job)
assert_equal "true", Resque.redis.get("lock:#{Job.lock}")
Resque.dequeue(Job)
assert_nil Resque.redis.get("lock:#{Job.lock}")
end
def test_unlock_with_args
Resque.enqueue(Job, 1)
assert_equal "true", Resque.redis.get("lock:#{Job.lock(1)}")
Resque.dequeue(Job, 1)
assert_nil Resque.redis.get("lock:#{Job.lock(1)}")
end

end