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

Delete runtime locks on exception #382

Merged
merged 17 commits into from Apr 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .rubocop.yml
@@ -1,4 +1,5 @@
require:
- rubocop-performance
- rubocop-rspec

inherit_mode:
Expand Down Expand Up @@ -64,6 +65,9 @@ Naming/FileName:
Exclude:
- lib/sidekiq-unique-jobs.rb

Naming/RescuedExceptionsVariableName:
PreferredName: ex

Naming/UncommunicativeMethodParamName:
AllowedNames:
- ex
Expand Down
11 changes: 5 additions & 6 deletions .travis.yml
Expand Up @@ -25,22 +25,21 @@ script:

after_script:
- if [[ "${COV}" = "true" ]]; then ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT; fi;

rvm:
- 2.6.1
- 2.6.2
- 2.4.5
- 2.3.8
- jruby-9.2.5.0
- jruby-9.2.7.0

matrix:
fast_finish: true
include:
- rvm: 2.5.3
- rvm: 2.5.5
gemfile: gemfiles/sidekiq_develop.gemfile
env: COV=true
- rvm: 2.6.0
- rvm: 2.6.2
gemfile: gemfiles/sidekiq_6.0.gemfile
- rvm: 2.5.3
- rvm: 2.5.5
gemfile: gemfiles/sidekiq_6.0.gemfile

gemfile:
Expand Down
2 changes: 2 additions & 0 deletions Gemfile
Expand Up @@ -19,8 +19,10 @@ platforms :mri_25 do
gem "guard-rubocop"
gem "memory_profiler"
gem "pry"
gem "redcarpet", "~> 3.4"
gem "reek", ">= 5.3"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
14 changes: 12 additions & 2 deletions README.md
Expand Up @@ -5,6 +5,8 @@
- [Introduction](#introduction)
- [Documentation](#documentation)
- [Requirements](#requirements)
- [ActiveJob](#activejob)
- [redis-namespace](#redis-namespace)
- [Installation](#installation)
- [Support Me](#support-me)
- [General Information](#general-information)
Expand Down Expand Up @@ -56,9 +58,17 @@ Below are links to the latest major versions (4 & 5):

## Requirements

See [Sidekiq requirements][] for what is required. Starting from 5.0.0 only sidekiq >= 4 is supported and support for MRI <= 2.1 is dropped. ActiveJob is not supported
See [Sidekiq requirements][] for what is required. Starting from 5.0.0 only sidekiq >= 4 and MRI >= 2.2. ActiveJob is not supported

Version 6 requires Redis >= 3 and pure Sidekiq, no ActiveJob supported anymore. See [About ActiveJob](https://github.com/mhenrixon/sidekiq-unique-jobs/wiki/About-ActiveJob) for why.
### ActiveJob

Version 6 requires Redis >= 3 and pure Sidekiq, no ActiveJob supported anymore. See [About ActiveJob](https://github.com/mhenrixon/sidekiq-unique-jobs/wiki/About-ActiveJob) for why. It simply is too complex and generates more issues than I can handle given how little timer I have to spend on this project.

### redis-namespace

Will not be officially supported anymore. Since Mike [won't support redis-namespace](https://github.com/mperham/sidekiq/issues/3366#issuecomment-284270120) neither will I.

[Read this](http://www.mikeperham.com/2017/04/10/migrating-from-redis-namespace/) for how to migrate away from namespacing.

## Installation

Expand Down
22 changes: 19 additions & 3 deletions bin/bench
Expand Up @@ -10,11 +10,27 @@ require "benchmark/ips"
require "sidekiq-unique-jobs"

ITERATIONS ||= 10_000
LOCK_TYPES = %w[
until_executed
until_expired
until_and_while_executing
until_executing
while_executing
].freeze

Benchmark.ips do |x|
x.config(time: 5, warmup: 2)
x.report("new_shit") do |_times|
SidekiqUniqueJobs::Scripts::AcquireLock.execute(nil, SecureRandom.hex, SecureRandom.hex)
x.config(time: 180, warmup: 2)
x.report("locksmith lock and unlock") do |_times|
item = {
"ttl" => rand(10),
"jid" => SecureRandom.hex,
"unique_digest" => SecureRandom.hex,
"lock" => LOCK_TYPES.sample,
}
locksmith = SidekiqUniqueJobs::Locksmith.new(item)
locksmith.lock
locksmith.unlock
end

x.compare!
end
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_4.0.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_4.1.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_4.2.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_5.0.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_5.1.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_5.2.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_6.0.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/sidekiq_develop.gemfile
Expand Up @@ -19,7 +19,9 @@ platforms :mri_25 do
gem "memory_profiler"
gem "pry"
gem "reek", ">= 5.3"
gem "redcarpet", "~> 3.4"
gem "rubocop"
gem "rubocop-performance"
gem "rubocop-rspec"
gem "simplecov-json"
gem "travis"
Expand Down
1 change: 0 additions & 1 deletion lib/sidekiq_unique_jobs/lock/until_and_while_executing.rb
Expand Up @@ -19,7 +19,6 @@ def execute
return unless locked?

unlock

runtime_lock.execute { yield }
end

Expand Down
8 changes: 7 additions & 1 deletion lib/sidekiq_unique_jobs/lock/while_executing.rb
Expand Up @@ -16,6 +16,7 @@ class WhileExecuting < BaseLock
# @param [Hash] item the Sidekiq job hash
# @param [Proc] callback callback to call after unlock
# @param [Sidekiq::RedisConnection, ConnectionPool] redis_pool the redis connection
#
def initialize(item, callback, redis_pool = nil)
super(item, callback, redis_pool)
append_unique_key_suffix
Expand All @@ -34,7 +35,12 @@ def lock
def execute
return strategy.call unless locksmith.lock(item[LOCK_TIMEOUT_KEY])

with_cleanup { yield }
yield
rescue Exception # rubocop:disable Lint/RescueException
delete!
raise
else
unlock_with_callback
end

private
Expand Down
8 changes: 0 additions & 8 deletions lib/sidekiq_unique_jobs/lock/while_executing_reject.rb
Expand Up @@ -11,14 +11,6 @@ class Lock
#
# @author Mikael Henriksson <mikael@zoolutions.se>
class WhileExecutingReject < WhileExecuting
# Executes in the Sidekiq server process
# @yield to the worker class perform method
def execute
return strategy.call unless locksmith.lock(item[LOCK_TIMEOUT_KEY])

with_cleanup { yield }
end

# Overridden with a forced {OnConflict::Reject} strategy
# @return [OnConflict::Reject] a reject strategy
def strategy
Expand Down
29 changes: 25 additions & 4 deletions lib/sidekiq_unique_jobs/locksmith.rb
Expand Up @@ -23,7 +23,10 @@ def initialize(item, redis_pool = nil)
@redis_pool = redis_pool
end

#
# Deletes the lock unless it has a ttl set
#
#
def delete
return if ttl

Expand All @@ -39,11 +42,14 @@ def delete!
)
end

#
# Create a lock for the item
#
# @param [Integer] timeout the number of seconds to wait for a lock.
# nil means wait indefinitely
# @yield the block to execute if a lock is successful
# @return the Sidekiq job_id (jid)
#
# @return [String] the Sidekiq job_id (jid)
#
#
def lock(timeout = nil, &block)
Scripts.call(:lock, redis_pool,
keys: [exists_key, grabbed_key, available_key, UNIQUE_SET, unique_digest],
Expand All @@ -56,19 +62,27 @@ def lock(timeout = nil, &block)
end
alias wait lock

#
# Removes the lock keys from Redis if locked by the provided jid/token
#
# @return [false] unless locked?
# @return [String] Sidekiq job_id (jid) if successful
#
def unlock(token = nil)
token ||= jid
return false unless locked?(token)

unlock!(token)
end

#
# Removes the lock keys from Redis
#
# @param [String] token the token to unlock (defaults to jid)
#
# @return [false] unless locked?
# @return [String] Sidekiq job_id (jid) if successful
#
def unlock!(token = nil)
token ||= jid

Expand All @@ -80,10 +94,17 @@ def unlock!(token = nil)
)
end

# Checks if this instance is considered locked
#
# @param [String] token the unique token to check for a lock.
# nil will default to the jid provided in the initializer
# @return [true, false]
#
# Checks if this instance is considered locked
#
# @param [<type>] token <description>
#
# @return [<type>] <description>
#
def locked?(token = nil)
token ||= jid

Expand Down