diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f6769fa78..44e59a531 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -10,7 +10,9 @@ jobs: - uses: actions/checkout@v2 - uses: ruby/setup-ruby@v1 with: - ruby-version: 2.6 + ruby-version: 2.7 + bundler: 2.2.5 + bundler-cache: true - run: bin/bundle --jobs=$(nproc) --retry=$(nproc) - run: bin/rubocop -P @@ -23,6 +25,8 @@ jobs: - uses: actions/checkout@v2 - uses: ruby/setup-ruby@v1 with: - ruby-version: 2.6 + ruby-version: 2.7 + bundler: 2.2.5 + bundler-cache: true - run: bin/bundle --jobs=$(nproc) --retry=$(nproc) - run: bin/reek . diff --git a/.github/workflows/rspec.yml b/.github/workflows/rspec.yml index c0a8880d7..4984f1eac 100644 --- a/.github/workflows/rspec.yml +++ b/.github/workflows/rspec.yml @@ -14,20 +14,27 @@ jobs: - uses: actions/checkout@v2 - uses: ruby/setup-ruby@v1 with: - ruby-version: 2.6.5 - - run: bin/bundle --jobs=$(nproc) --retry=$(nproc) - - name: Coverage - uses: paambaati/codeclimate-action@v2.6.0 + ruby-version: 2.7 + bundler: 2.2.5 + bundler-cache: true + + - name: Install Code Climate reporter + run: | + sudo curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter + sudo chmod +x ./cc-test-reporter + - name: Generate Coverage env: - COV: true CC_TEST_REPORTER_ID: 88e524e8f638efe690def7a6e2c72b1a9db5cdfa74548921b734d609a5858ee5 - with: - coverageCommand: bin/rspec --require spec_helper --tag ~perf - debug: true - tests: - env: - COV: false + run: | + export GIT_BRANCH=${GITHUB_REF#refs/heads/} + export GIT_COMMIT_SHA=${GITHUB_SHA} + echo $GIT_BRANCH + echo $GIT_COMMIT_SHA + ./cc-test-reporter before-build + COV=true bin/rspec --require spec_helper --tag ~perf + ./cc-test-reporter after-build --coverage-input-type simplecov --exit-code $? + tests: services: redis: image: redis:latest @@ -39,7 +46,6 @@ jobs: strategy: fail-fast: true matrix: - # ruby: [2.5, 2.6, 2.7, jruby, truffleruby] ruby: [2.5, 2.6, 2.7] steps: @@ -47,6 +53,7 @@ jobs: - uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby }} - - run: bin/bundle --jobs=$(nproc) --retry=$(nproc) - - run: bin/appraisal install --jobs=$(nproc) --retry=$(nproc) + bundler: 2.2.5 + bundler-cache: true + - run: bin/appraisal install --jobs=$(nproc) --retry=$(nproc) - run: bin/appraisal rspec --require spec_helper --tag ~perf diff --git a/.rubocop.yml b/.rubocop.yml index c58c913dd..d41d6b7b8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -87,6 +87,7 @@ RSpec/ContextWording: - unless - for - that + - and RSpec/DescribeClass: Exclude: diff --git a/.simplecov b/.simplecov index d74028a63..2a23d626c 100644 --- a/.simplecov +++ b/.simplecov @@ -1,13 +1,18 @@ # frozen_string_literal: true -require "simplecov-oj" +require "simplecov_json_formatter" +require "simplecov-sublime" -SimpleCov.command_name "RSpec" -# SimpleCov.refuse_coverage_drop -SimpleCov.formatters = [ +CI_FORMATTERS = [ + SimpleCov::Formatter::SimpleFormatter, + SimpleCov::Formatter::JSONFormatter, +].freeze + +LOCAL_FORMATTERS = [ + SimpleCov::Formatter::JSONFormatter, + SimpleCov::Formatter::SublimeFormatter, SimpleCov::Formatter::HTMLFormatter, - SimpleCov::Formatter::OjFormatter, -] +].freeze SimpleCov.start do add_filter "/spec/" @@ -15,10 +20,26 @@ SimpleCov.start do add_filter "/gemfiles/" add_filter "/lib/sidekiq/" add_filter "/lib/sidekiq_unique_jobs/testing.rb" + add_filter "/myapp/" add_filter "/lib/sidekiq_unique_jobs/core_ext.rb" add_group "Locks", "lib/sidekiq_unique_jobs/lock" add_group "Middelware", "lib/sidekiq_unique_jobs/middleware" add_group "Redis", "lib/sidekiq_unique_jobs/redis" add_group "Timeout", "lib/sidekiq_unique_jobs/timeout" + + enable_coverage :branch + primary_coverage :branch + + if ENV["CI"] + formatter SimpleCov::Formatter::MultiFormatter.new(CI_FORMATTERS) + else + formatter SimpleCov::Formatter::MultiFormatter.new(LOCAL_FORMATTERS) + + refuse_coverage_drop + minimum_coverage line: 90, branch: 80 + minimum_coverage_by_file line: 90, branch: 80 + end + + track_files "**/*.rb" end diff --git a/Appraisals b/Appraisals index 3a5832ec1..af1b5b92e 100644 --- a/Appraisals +++ b/Appraisals @@ -4,18 +4,6 @@ appraise "sidekiq-develop" do gem "sidekiq", git: "https://github.com/mperham/sidekiq.git" end -appraise "sidekiq-4.0" do - gem "sidekiq", "~> 4.0.0" -end - -appraise "sidekiq-4.1" do - gem "sidekiq", "~> 4.1.0" -end - -appraise "sidekiq-4.2" do - gem "sidekiq", "~> 4.2.0" -end - appraise "sidekiq-5.0" do gem "sidekiq", "~> 5.0.0" end @@ -29,5 +17,9 @@ appraise "sidekiq-5.2" do end appraise "sidekiq-6.0" do - gem "sidekiq", ">= 6.0.pre", "< 6.1" + gem "sidekiq", "~> 6.0.0" +end + +appraise "sidekiq-6.1" do + gem "sidekiq", "~> 6.1.0" end diff --git a/Gemfile b/Gemfile index 696874c0e..575964f15 100644 --- a/Gemfile +++ b/Gemfile @@ -30,8 +30,7 @@ platforms :mri do gem "reek", ">= 5.3" gem "rspec-benchmark" gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" + gem "simplecov-sublime", "0.21.0", require: false gem "travis" end diff --git a/README.md b/README.md index f07463a58..8d91c02dc 100644 --- a/README.md +++ b/README.md @@ -3,9 +3,11 @@ - [Introduction](#introduction) -- [Requirements](#requirements) -- [Installation](#installation) +- [Usage](#usage) + - [Installation](#installation) + - [Your first worker](#your-first-worker) - [Support Me](#support-me) +- [Requirements](#requirements) - [General Information](#general-information) - [Global Configuration](#global-configuration) - [debug_lua](#debug_lua) @@ -21,6 +23,8 @@ - [lock_prefix](#lock_prefix) - [lock_info](#lock_info) - [Worker Configuration](#worker-configuration) + - [lock_info](#lock_info-1) + - [lock_prefix](#lock_prefix-1) - [lock_ttl](#lock_ttl-1) - [lock_timeout](#lock_timeout-1) - [unique_across_queues](#unique_across_queues) @@ -39,7 +43,7 @@ - [replace](#replace) - [Reschedule](#reschedule) - [Custom Strategies](#custom-strategies) -- [Usage](#usage) +- [Usage](#usage-1) - [Finer Control over Uniqueness](#finer-control-over-uniqueness) - [After Unlock Callback](#after-unlock-callback) - [Logging](#logging) @@ -61,7 +65,7 @@ ## Introduction -This gem adds unique constraints to the sidekiq queues. The uniqueness is achieved by acquiring locks for a hash of a queue name, a worker class, and job's arguments. Only one lock for a given hash can be acquired. What happens when a lock can't be acquired is governed by a chosen strategy. +This gem adds unique constraints to the sidekiq queues. The uniqueness is achieved by acquiring locks for a hash of a queue name, a worker class, and job's arguments. By default, only one lock for a given hash can be acquired. What happens when a lock can't be acquired is governed by a chosen `on_conflict`strategy. This is the documentation for the master branch. You can find the documentation for each release by navigating to its tag. @@ -71,20 +75,9 @@ Here are links to some of the old versions - [v5.0.10](https://github.com/mhenrixon/sidekiq-unique-jobs/tree/v5.0.10) - [v4.0.18](https://github.com/mhenrixon/sidekiq-unique-jobs/tree/v4.0.18) -## Requirements - -- Sidekiq `>= 4.0` (`>= 5.2` recommended) -- Ruby: - - MRI `>= 2.3` (`>= 2.5` recommended) - - JRuby `>= 9.0` (`>= 9.2` recommended) - - Truffleruby -- Redis Server `>= 3.0.2` (`>= 3.2` recommended) -- [ActiveJob officially not supported][48] -- [redis-namespace officially not supported][49] - -See [Sidekiq requirements][24] for detailed requirements of Sidekiq itself (be sure to check the right sidekiq version). +## Usage -## Installation +### Installation Add this line to your application's Gemfile: @@ -98,16 +91,51 @@ And then execute: bundle ``` -Or install it yourself as: +### Your first worker + +```ruby +# frozen_string_literal: true + +class UntilExecutedWorker + include Sidekiq::Worker + + sidekiq_options queue: :special, + retry: false, + lock: :until_executed, + lock_info: true, + lock_timeout: 0, + lock_prefix: "special", + lock_ttl: 0, + lock_limit: 5 + + def perform + logger.info("cowboy") + sleep(1) # hardcore processing + logger.info("beebop") + end +end -```bash -gem install sidekiq-unique-jobs ``` +You can read more about the worker configuration in [Worker Configuration](#worker-configuration) below. + ## Support Me Want to show me some ❤️ for the hard work I do on this gem? You can use the following PayPal link: [https://paypal.me/mhenrixon1](https://paypal.me/mhenrixon1). Any amount is welcome and let me tell you it feels good to be appreciated. Even a dollar makes me super excited about all of this. +## Requirements + +- Sidekiq `>= 4.0` (`>= 5.2` recommended) +- Ruby: + - MRI `>= 2.5` (`>= 2.6` recommended) + - JRuby `>= 9.0` (`>= 9.2` recommended) + - Truffleruby +- Redis Server `>= 3.0.2` (`>= 3.2` recommended) +- [ActiveJob officially not supported][48] +- [redis-namespace officially not supported][49] + +See [Sidekiq requirements][24] for detailed requirements of Sidekiq itself (be sure to check the right sidekiq version). + ## General Information See [Interaction w/ Sidekiq](https://github.com/mhenrixon/sidekiq-unique-jobs/wiki/How-this-gem-interacts-with-Sidekiq) on how the gem interacts with Sidekiq. @@ -253,11 +281,27 @@ Using lock info will create an additional key for the lock with a json object co ## Worker Configuration +### lock_info + +Lock info gathers information about a specific lock. It collects things like which `lock_args` where used to compute the `lock_digest` that is used for maintaining uniqueness. + +```ruby +sidekiq_options lock_info: false # this is the default, set to true to turn on +``` + +### lock_prefix + +Use if you want a different key prefix for the keys in redis. + +```ruby +sidekiq_options lock_prefix: "uniquejobs" # this is the default value +``` + ### lock_ttl Lock TTL decides how long to wait after the job has been successfully processed before making it possible to reuse that lock. -Since `v6.0.11` the other locks will expire after the server is done processing. +Starting from `v7` the expiration will take place when the job is pushed to the queue. ```ruby sidekiq_options lock_ttl: nil # default - don't expire keys @@ -278,6 +322,8 @@ sidekiq_options lock_timeout: nil # lock indefinitely, this process won't contin This configuration option is slightly misleading. It doesn't disregard the queue on other jobs. Just on itself, this means that a worker that might schedule jobs into multiple queues will be able to have uniqueness enforced on all queues it is pushed to. +This is mainly intended for `Worker.set(queue: :another).perform_async`. + ```ruby class Worker include Sidekiq::Worker diff --git a/gemfiles/sidekiq_4.1.gemfile b/gemfiles/sidekiq_4.1.gemfile deleted file mode 100644 index d3c841f54..000000000 --- a/gemfiles/sidekiq_4.1.gemfile +++ /dev/null @@ -1,36 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "appraisal" -gem "bundler" -gem "gem-release" -gem "github-markup" -gem "rack-test" -gem "rake" -gem "rspec" -gem "rspec-its" -gem "sinatra" -gem "timecop" -gem "yard" -gem "sidekiq", "~> 4.1.0" - -platforms :mri do - gem "fasterer" - gem "github_changelog_generator" - gem "guard" - gem "guard-bundler" - gem "guard-reek" - gem "guard-rspec" - gem "guard-rubocop" - gem "hiredis" - gem "redcarpet", "~> 3.4" - gem "reek", ">= 5.3" - gem "rspec-benchmark" - gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" - gem "travis" -end - -gemspec path: "../" diff --git a/gemfiles/sidekiq_4.2.gemfile b/gemfiles/sidekiq_4.2.gemfile deleted file mode 100644 index 32f9954dc..000000000 --- a/gemfiles/sidekiq_4.2.gemfile +++ /dev/null @@ -1,36 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "appraisal" -gem "bundler" -gem "gem-release" -gem "github-markup" -gem "rack-test" -gem "rake" -gem "rspec" -gem "rspec-its" -gem "sinatra" -gem "timecop" -gem "yard" -gem "sidekiq", "~> 4.2.0" - -platforms :mri do - gem "fasterer" - gem "github_changelog_generator" - gem "guard" - gem "guard-bundler" - gem "guard-reek" - gem "guard-rspec" - gem "guard-rubocop" - gem "hiredis" - gem "redcarpet", "~> 3.4" - gem "reek", ">= 5.3" - gem "rspec-benchmark" - gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" - gem "travis" -end - -gemspec path: "../" diff --git a/gemfiles/sidekiq_5.0.gemfile b/gemfiles/sidekiq_5.0.gemfile index 2322ffc21..71e4186dc 100644 --- a/gemfiles/sidekiq_5.0.gemfile +++ b/gemfiles/sidekiq_5.0.gemfile @@ -28,8 +28,7 @@ platforms :mri do gem "reek", ">= 5.3" gem "rspec-benchmark" gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" + gem "simplecov-sublime", "0.21.0", require: false gem "travis" end diff --git a/gemfiles/sidekiq_5.1.gemfile b/gemfiles/sidekiq_5.1.gemfile index 8972d8d5f..c32cfbd22 100644 --- a/gemfiles/sidekiq_5.1.gemfile +++ b/gemfiles/sidekiq_5.1.gemfile @@ -28,8 +28,7 @@ platforms :mri do gem "reek", ">= 5.3" gem "rspec-benchmark" gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" + gem "simplecov-sublime", "0.21.0", require: false gem "travis" end diff --git a/gemfiles/sidekiq_5.2.gemfile b/gemfiles/sidekiq_5.2.gemfile index 5ef6615f8..08fa30e31 100644 --- a/gemfiles/sidekiq_5.2.gemfile +++ b/gemfiles/sidekiq_5.2.gemfile @@ -28,8 +28,7 @@ platforms :mri do gem "reek", ">= 5.3" gem "rspec-benchmark" gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" + gem "simplecov-sublime", "0.21.0", require: false gem "travis" end diff --git a/gemfiles/sidekiq_6.0.gemfile b/gemfiles/sidekiq_6.0.gemfile index 2be3f4cac..89cde0dea 100644 --- a/gemfiles/sidekiq_6.0.gemfile +++ b/gemfiles/sidekiq_6.0.gemfile @@ -13,7 +13,7 @@ gem "rspec-its" gem "sinatra" gem "timecop" gem "yard" -gem "sidekiq", ">= 6.0.pre", "< 6.1" +gem "sidekiq", "~> 6.0.0" platforms :mri do gem "fasterer" @@ -28,8 +28,7 @@ platforms :mri do gem "reek", ">= 5.3" gem "rspec-benchmark" gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" + gem "simplecov-sublime", "0.21.0", require: false gem "travis" end diff --git a/gemfiles/sidekiq_4.0.gemfile b/gemfiles/sidekiq_6.1.gemfile similarity index 87% rename from gemfiles/sidekiq_4.0.gemfile rename to gemfiles/sidekiq_6.1.gemfile index cc50d08e8..04c4a2b9d 100644 --- a/gemfiles/sidekiq_4.0.gemfile +++ b/gemfiles/sidekiq_6.1.gemfile @@ -13,7 +13,7 @@ gem "rspec-its" gem "sinatra" gem "timecop" gem "yard" -gem "sidekiq", "~> 4.0.0" +gem "sidekiq", "~> 6.1.0" platforms :mri do gem "fasterer" @@ -28,8 +28,7 @@ platforms :mri do gem "reek", ">= 5.3" gem "rspec-benchmark" gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" + gem "simplecov-sublime", "0.21.0", require: false gem "travis" end diff --git a/gemfiles/sidekiq_develop.gemfile b/gemfiles/sidekiq_develop.gemfile index 23b4fcba9..a5f1b6e36 100644 --- a/gemfiles/sidekiq_develop.gemfile +++ b/gemfiles/sidekiq_develop.gemfile @@ -28,8 +28,7 @@ platforms :mri do gem "reek", ">= 5.3" gem "rspec-benchmark" gem "rubocop-mhenrixon" - gem "simplecov", "< 0.18" - gem "simplecov-oj" + gem "simplecov-sublime", "0.21.0", require: false gem "travis" end diff --git a/lib/sidekiq_unique_jobs/changelog.rb b/lib/sidekiq_unique_jobs/changelog.rb index a5a2f05bf..24aaa5e06 100644 --- a/lib/sidekiq_unique_jobs/changelog.rb +++ b/lib/sidekiq_unique_jobs/changelog.rb @@ -36,7 +36,7 @@ def add(message:, digest:, job_id:, script:) # def entries(pattern: "*", count: nil) options = {} - options[:match] = pattern if pattern + options[:match] = pattern options[:count] = count if count redis do |conn| diff --git a/lib/sidekiq_unique_jobs/constants.rb b/lib/sidekiq_unique_jobs/constants.rb index e286e9f47..cff33ddbc 100644 --- a/lib/sidekiq_unique_jobs/constants.rb +++ b/lib/sidekiq_unique_jobs/constants.rb @@ -11,6 +11,7 @@ module SidekiqUniqueJobs AT ||= "at" CHANGELOGS ||= "uniquejobs:changelog" CLASS ||= "class" + CREATED_AT ||= "created_at" DEAD_VERSION ||= "uniquejobs:dead" DIGESTS ||= "uniquejobs:digests" ERRORS ||= "errors" diff --git a/lib/sidekiq_unique_jobs/digests.rb b/lib/sidekiq_unique_jobs/digests.rb index 8a271937b..0e8a3e813 100644 --- a/lib/sidekiq_unique_jobs/digests.rb +++ b/lib/sidekiq_unique_jobs/digests.rb @@ -78,7 +78,7 @@ def delete_by_digest(digest) # rubocop:disable Metrics/MethodLength def entries(pattern: SCAN_PATTERN, count: DEFAULT_COUNT) options = {} options[:match] = pattern - options[:count] = count if count + options[:count] = count result = redis { |conn| conn.zscan_each(key, **options).to_a } diff --git a/lib/sidekiq_unique_jobs/lock_config.rb b/lib/sidekiq_unique_jobs/lock_config.rb index 11fb286b3..ae4d7c96b 100644 --- a/lib/sidekiq_unique_jobs/lock_config.rb +++ b/lib/sidekiq_unique_jobs/lock_config.rb @@ -98,6 +98,8 @@ def valid? # @return [String] # def errors_as_string + return if valid? + @errors_as_string ||= begin error_msg = +"\t" error_msg << errors.map { |key, val| "#{key}: :#{val}" }.join("\n\t") diff --git a/lib/sidekiq_unique_jobs/lua/lock.lua b/lib/sidekiq_unique_jobs/lua/lock.lua index 38056bc3a..d0df7e38e 100644 --- a/lib/sidekiq_unique_jobs/lua/lock.lua +++ b/lib/sidekiq_unique_jobs/lua/lock.lua @@ -69,8 +69,7 @@ redis.call("LREM", queued, -1, job_id) log_debug("LREM", primed, 1, job_id) redis.call("LREM", primed, 1, job_id) --- The Sidekiq client should only set pttl for until_expired --- The Sidekiq server should set pttl for all other jobs +-- The Sidekiq client sets pttl if pttl and pttl > 0 then log_debug("PEXPIRE", digest, pttl) redis.call("PEXPIRE", digest, pttl) diff --git a/lib/sidekiq_unique_jobs/on_conflict/replace.rb b/lib/sidekiq_unique_jobs/on_conflict/replace.rb index 14244cb78..d434da9e7 100644 --- a/lib/sidekiq_unique_jobs/on_conflict/replace.rb +++ b/lib/sidekiq_unique_jobs/on_conflict/replace.rb @@ -11,9 +11,9 @@ class Replace < OnConflict::Strategy # @return [String] rthe sidekiq queue this job belongs to attr_reader :queue # - # @!attribute [r] unique_digest + # @!attribute [r] lock_digest # @return [String] the unique digest to use for locking - attr_reader :unique_digest + attr_reader :lock_digest # # Initialize a new Replace strategy @@ -22,8 +22,8 @@ class Replace < OnConflict::Strategy # def initialize(item, redis_pool = nil) super(item, redis_pool) - @queue = item[QUEUE] - @unique_digest = item[LOCK_DIGEST] + @queue = item[QUEUE] + @lock_digest = item[LOCK_DIGEST] end # @@ -37,10 +37,11 @@ def initialize(item, redis_pool = nil) def call(&block) return unless (deleted_job = delete_job_by_digest) - log_info("Deleting job: #{deleted_job}") + log_info("Deleted job: #{deleted_job}") if (del_count = delete_lock) - log_info("Deleted `#{del_count}` keys for #{unique_digest}") + log_info("Deleted `#{del_count}` keys for #{lock_digest}") end + block&.call end @@ -54,7 +55,7 @@ def call(&block) def delete_job_by_digest call_script(:delete_job_by_digest, keys: ["#{QUEUE}:#{queue}", SCHEDULE, RETRY], - argv: [unique_digest]) + argv: [lock_digest]) end # @@ -64,7 +65,7 @@ def delete_job_by_digest # @return [Integer] the number of keys deleted # def delete_lock - digests.delete_by_digest(unique_digest) + digests.delete_by_digest(lock_digest) end # diff --git a/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb b/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb index 4e32691c9..7f7e753e0 100644 --- a/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb +++ b/lib/sidekiq_unique_jobs/on_conflict/reschedule.rb @@ -21,7 +21,7 @@ def initialize(item, redis_pool = nil) def call if sidekiq_worker_class? log_info("Rescheduling #{item[LOCK_DIGEST]}") - worker_class&.perform_in(5, *item[ARGS]) + worker_class.perform_in(5, *item[ARGS]) else log_warn("Skip rescheduling of #{item[LOCK_DIGEST]} because #{worker_class} is not a Sidekiq::Worker") end diff --git a/lib/sidekiq_unique_jobs/orphans/manager.rb b/lib/sidekiq_unique_jobs/orphans/manager.rb index d98424f91..51a38ff4b 100644 --- a/lib/sidekiq_unique_jobs/orphans/manager.rb +++ b/lib/sidekiq_unique_jobs/orphans/manager.rb @@ -59,7 +59,12 @@ def stop # @return [] # def task - @task ||= Concurrent::TimerTask.new(timer_task_options) do + @task ||= Concurrent::TimerTask.new(timer_task_options, &task_body) + end + + # @private + def task_body + @task_body ||= lambda do with_logging_context do redis do |conn| refresh_reaper_mutex diff --git a/lib/sidekiq_unique_jobs/profiler.rb b/lib/sidekiq_unique_jobs/profiler.rb deleted file mode 100644 index 565a3aef0..000000000 --- a/lib/sidekiq_unique_jobs/profiler.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -module SidekiqUniqueJobs - # - # Class MethodProfiler provides method level profiling - # - # @author Mikael Henriksson - # - class Profiler - def self.patch(klass, methods, name) # rubocop:disable Metrics/MethodLength - patches = methods.map do |method_name| - <<~RUBY - unless defined?(#{method_name}__mp_unpatched) - alias_method :#{method_name}__mp_unpatched, :#{method_name} - def #{method_name}(*args, &blk) - unless prof = Thread.current[:_method_profiler] - return #{method_name}__mp_unpatched(*args, &blk) - end - begin - start = Process.clock_gettime(Process::CLOCK_MONOTONIC) - #{method_name}__mp_unpatched(*args, &blk) - ensure - data = (prof[:#{name}] ||= {duration: 0.0, calls: 0}) - data[:duration] += Process.clock_gettime(Process::CLOCK_MONOTONIC) - start - data[:calls] += 1 - end - end - end - RUBY - end.join("\n") - - klass.class_eval patches - end - - def self.start - Thread.current[:_method_profiler] = { - __start: current_timestamp, - } - end - - def self.stop - finish = current_timestamp - return unless (data = Thread.current[:_method_profiler]) - - Thread.current[:_method_profiler] = nil - start = data.delete(:__start) - data[:total_duration] = finish - start - data - end - - def self.current_timestamp - Process.clock_gettime(Process::CLOCK_MONOTONIC) - end - end -end diff --git a/lib/sidekiq_unique_jobs/redis/entity.rb b/lib/sidekiq_unique_jobs/redis/entity.rb index 6b47034c2..ef274c47c 100644 --- a/lib/sidekiq_unique_jobs/redis/entity.rb +++ b/lib/sidekiq_unique_jobs/redis/entity.rb @@ -48,10 +48,10 @@ def initialize(key) def exist? redis do |conn| value = conn.exists(key) - return true if value.is_a?(TrueClass) - return false if value.is_a?(FalseClass) - value.positive? + return value if boolean?(value) + + value.to_i.positive? end end @@ -95,6 +95,12 @@ def expires? def count 0 end + + private + + def boolean?(value) + [TrueClass, FalseClass].any? { |klazz| value.is_a?(klazz) } + end end end end diff --git a/lib/sidekiq_unique_jobs/version.rb b/lib/sidekiq_unique_jobs/version.rb index 82eabd9da..de1a07125 100644 --- a/lib/sidekiq_unique_jobs/version.rb +++ b/lib/sidekiq_unique_jobs/version.rb @@ -3,5 +3,5 @@ module SidekiqUniqueJobs # # @return [String] the current SidekiqUniqueJobs version - VERSION = "7.0.0.beta29.1" + VERSION = "7.0.0" end diff --git a/lib/sidekiq_unique_jobs/web.rb b/lib/sidekiq_unique_jobs/web.rb index 8db079068..6d7bcbf79 100644 --- a/lib/sidekiq_unique_jobs/web.rb +++ b/lib/sidekiq_unique_jobs/web.rb @@ -53,7 +53,13 @@ def self.registered(app) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize end end -require "sidekiq/web" unless defined?(Sidekiq::Web) -Sidekiq::Web.register(SidekiqUniqueJobs::Web) -Sidekiq::Web.tabs["Locks"] = "locks" -Sidekiq::Web.settings.locales << File.join(File.dirname(__FILE__), "locales") +begin + require "delegate" unless defined?(DelegateClass) + require "sidekiq/web" unless defined?(Sidekiq::Web) + + Sidekiq::Web.register(SidekiqUniqueJobs::Web) + Sidekiq::Web.tabs["Locks"] = "locks" + Sidekiq::Web.settings.locales << File.join(File.dirname(__FILE__), "locales") +rescue NameError, LoadError => ex + SidekiqUniqueJobs.logger.error(ex) +end diff --git a/myapp/app/workers/until_executed_job.rb b/myapp/app/workers/until_executed_job.rb index f3c2fa0db..7e43fdf56 100644 --- a/myapp/app/workers/until_executed_job.rb +++ b/myapp/app/workers/until_executed_job.rb @@ -11,7 +11,7 @@ class UntilExecutedJob def perform logger.info("cowboy") - sleep(1) + sleep(1) # hardcore processing logger.info("beebop") end end diff --git a/sidekiq-unique-jobs.gemspec b/sidekiq-unique-jobs.gemspec index 9d0bef6ce..634f92d01 100644 --- a/sidekiq-unique-jobs.gemspec +++ b/sidekiq-unique-jobs.gemspec @@ -69,6 +69,6 @@ Gem::Specification.new do |spec| spec.add_dependency "brpoplpush-redis_script", "> 0.0.0", "<= 2.0.0" spec.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.5" - spec.add_dependency "sidekiq", ">= 4.0", "< 7.0" + spec.add_dependency "sidekiq", ">= 5.0", "< 7.0" spec.add_dependency "thor", ">= 0.20", "< 2.0" end diff --git a/spec/sidekiq_unique_jobs/changelog_spec.rb b/spec/sidekiq_unique_jobs/changelog_spec.rb index 8075ecbfa..cc6b305ec 100644 --- a/spec/sidekiq_unique_jobs/changelog_spec.rb +++ b/spec/sidekiq_unique_jobs/changelog_spec.rb @@ -116,7 +116,10 @@ end describe "#entries" do - subject(:entries) { entity.entries } + subject(:entries) { entity.entries(pattern: pattern, count: count) } + + let(:pattern) { "*" } + let(:count) { nil } context "when no entries exist" do it { is_expected.to match_array([]) } @@ -145,6 +148,13 @@ end it { is_expected.to match_array([locked_entry, queued_entry]) } + + context "when given count 1" do + let(:count) { 1 } + + # count only is considered per iteration, this would have iterated twice + it { is_expected.to match_array([locked_entry, queued_entry]) } + end end end diff --git a/spec/sidekiq_unique_jobs/cli_spec.rb b/spec/sidekiq_unique_jobs/cli_spec.rb index 4a108ff23..bb6db17ea 100644 --- a/spec/sidekiq_unique_jobs/cli_spec.rb +++ b/spec/sidekiq_unique_jobs/cli_spec.rb @@ -3,7 +3,7 @@ require "thor/runner" require "irb" -RSpec.describe SidekiqUniqueJobs::Cli, ruby_ver: ">= 2.4" do +RSpec.describe SidekiqUniqueJobs::Cli, ruby_ver: ">= 2.5" do let(:item) do { "jid" => jid, diff --git a/spec/sidekiq_unique_jobs/lock/validator_spec.rb b/spec/sidekiq_unique_jobs/lock/validator_spec.rb index 183ffc086..57e6a571a 100644 --- a/spec/sidekiq_unique_jobs/lock/validator_spec.rb +++ b/spec/sidekiq_unique_jobs/lock/validator_spec.rb @@ -13,11 +13,49 @@ } end - it { expect(true).to eq(true) } - describe "#validate" do subject(:validate) { validator.validate } + context "when lock is until_executed" do + let(:options) do + { + "lock" => "until_executed", + } + end + + it { is_expected.to be_valid } + end + + context "when lock is until_executing" do + let(:options) do + { + "lock" => "until_executing", + } + end + + it { is_expected.to be_valid } + end + + context "when lock is while_executing" do + let(:options) do + { + "lock" => "while_executing", + } + end + + it { is_expected.to be_valid } + end + + context "when lock is until_and_while_executing" do + let(:options) do + { + "lock" => "until_and_while_executing", + } + end + + it { is_expected.to be_valid } + end + context "with deprecated sidekiq_options" do let(:options) do { diff --git a/spec/sidekiq_unique_jobs/lock_config_spec.rb b/spec/sidekiq_unique_jobs/lock_config_spec.rb index 13694d6b3..8601642a3 100644 --- a/spec/sidekiq_unique_jobs/lock_config_spec.rb +++ b/spec/sidekiq_unique_jobs/lock_config_spec.rb @@ -70,6 +70,18 @@ end end + describe "#errors_as_string" do + subject(:errors_as_string) { lock_config.errors_as_string } + + it { is_expected.to be_nil } + + context "when given errors" do + let(:errors) { { any: :thing } } + + it { is_expected.to eq("\tany: :thing") } + end + end + describe "#on_client_conflict" do subject(:on_client_conflict) { lock_config.on_client_conflict } diff --git a/spec/sidekiq_unique_jobs/on_conflict/replace_spec.rb b/spec/sidekiq_unique_jobs/on_conflict/replace_spec.rb index b35ff2c51..7bb162297 100644 --- a/spec/sidekiq_unique_jobs/on_conflict/replace_spec.rb +++ b/spec/sidekiq_unique_jobs/on_conflict/replace_spec.rb @@ -19,6 +19,66 @@ allow(block).to receive(:call) end + context "when delete_job_by_digest returns nil" do + let(:jid) { "abcdefab" } + + before do + allow(strategy).to receive(:delete_job_by_digest).and_return(nil) + end + + it { is_expected.to eq(nil) } + end + + context "when delete_lock returns 9" do + let(:jid) { "bogus" } + + before do + allow(strategy).to receive(:delete_job_by_digest).and_return(jid) + allow(strategy).to receive(:log_info).and_call_original + allow(strategy).to receive(:delete_lock).and_return(9) + end + + it "logs important information" do + call + + expect(strategy).to have_received(:log_info).with("Deleted job: #{jid}") + expect(strategy).to have_received(:log_info).with("Deleted `9` keys for #{lock_digest}") + end + end + + context "when delete_lock returns nil" do + let(:jid) { "bogus" } + + before do + allow(strategy).to receive(:delete_job_by_digest).and_return(jid) + allow(strategy).to receive(:log_info).and_call_original + allow(strategy).to receive(:delete_lock).and_return(nil) + end + + it "logs important information" do + call + + expect(strategy).to have_received(:log_info).with("Deleted job: #{jid}") + expect(strategy).not_to have_received(:log_info).with("Deleted `` keys for #{lock_digest}") + end + end + + context "when block is nil" do + let(:jid) { "bogus" } + let(:block) { nil } + + before do + allow(strategy).to receive(:delete_job_by_digest).and_return(jid) + allow(strategy).to receive(:log_info).and_call_original + allow(strategy).to receive(:delete_lock).and_return(nil) + end + + it "does not call block" do + call + expect(block).not_to have_received(:call) + end + end + context "when job is retried" do let(:jid) { "abcdefab" } let(:job) { dump_json(item) } diff --git a/spec/sidekiq_unique_jobs/on_conflict/reschedule_spec.rb b/spec/sidekiq_unique_jobs/on_conflict/reschedule_spec.rb index 72fee4fd8..bd95ef279 100644 --- a/spec/sidekiq_unique_jobs/on_conflict/reschedule_spec.rb +++ b/spec/sidekiq_unique_jobs/on_conflict/reschedule_spec.rb @@ -1,19 +1,41 @@ # frozen_string_literal: true RSpec.describe SidekiqUniqueJobs::OnConflict::Reschedule do - let(:strategy) { described_class.new(item) } - let(:unique_digest) { "uniquejobs:random-digest-value" } + let(:strategy) { described_class.new(item) } + let(:lock_digest) { "uniquejobs:random-digest-value" } + let(:worker_class) { "UniqueJobOnConflictReschedule" } let(:item) do - { "class" => UniqueJobOnConflictReschedule, - "lock_digest" => unique_digest, + { "class" => worker_class, + "lock_digest" => lock_digest, "args" => [1, 2] } end describe "#call" do let(:call) { strategy.call } - it do + before do + allow(UniqueJobOnConflictReschedule).to receive(:perform_in).and_call_original + end + + it "schedules a job five seconds from now" do expect { call }.to change { schedule_count }.by(1) + + expect(UniqueJobOnConflictReschedule).to have_received(:perform_in) + .with(5, *item["args"]) + end + + context "when not a sidekiq_worker_class?" do + before do + allow(strategy).to receive(:sidekiq_worker_class?).and_return(false) + allow(strategy).to receive(:log_warn).and_call_original + end + + it "logs a helpful warning" do + expect { call }.not_to change { schedule_count }.from(0) + + expect(strategy).to have_received(:log_warn) + .with("Skip rescheduling of #{lock_digest} because #{worker_class} is not a Sidekiq::Worker") + end end end diff --git a/spec/sidekiq_unique_jobs/orphans/manager_spec.rb b/spec/sidekiq_unique_jobs/orphans/manager_spec.rb index 3ec230b62..2265ea0f5 100644 --- a/spec/sidekiq_unique_jobs/orphans/manager_spec.rb +++ b/spec/sidekiq_unique_jobs/orphans/manager_spec.rb @@ -280,6 +280,56 @@ end end + describe ".refresh_reaper_mutex" do + subject(:refresh_reaper_mutex) { described_class.refresh_reaper_mutex } + + let(:frozen_time) { Time.new(1982, 6, 8, 14, 15, 34) } + + around do |example| + Timecop.freeze(frozen_time, &example) + end + + it "updates the redis key with timestamp" do + expect { refresh_reaper_mutex }.to change { get(SidekiqUniqueJobs::UNIQUE_REAPER) } + .from(nil).to(frozen_time.to_i.to_s) + + expect(ttl(SidekiqUniqueJobs::UNIQUE_REAPER)).to be_within(20).of(SidekiqUniqueJobs.config.reaper_interval) + end + end + + describe "#task" do + subject(:task) { described_class.task } + + before do + allow(Concurrent::TimerTask).to receive(:new).and_call_original + end + + it "initializes a new timer task with the correct arguments" do + expect(task).to be_a(Concurrent::TimerTask) + + expect(Concurrent::TimerTask).to have_received(:new) + .with(described_class.timer_task_options, &described_class.task_body) + end + end + + describe "#task_body" do + subject(:task_body) { described_class.task_body } + + before do + allow(described_class).to receive(:with_logging_context).and_yield + allow(described_class).to receive(:refresh_reaper_mutex).and_return(true) + allow(SidekiqUniqueJobs::Orphans::Reaper).to receive(:call).and_return(true) + end + + it "is wired up correctly" do + task_body.call + + expect(described_class).to have_received(:with_logging_context) + expect(described_class).to have_received(:refresh_reaper_mutex) + expect(SidekiqUniqueJobs::Orphans::Reaper).to have_received(:call) + end + end + describe ".logging_context" do subject(:logging_context) { described_class.logging_context } diff --git a/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb b/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb index c1d7a1244..e4e3d6af3 100644 --- a/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb +++ b/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb @@ -33,7 +33,9 @@ end describe ".call" do - subject(:call) { described_class.call } + subject(:call) { described_class.call(conn) } + + let(:conn) { nil } around do |example| SidekiqUniqueJobs.use_config(reaper: reaper) do @@ -41,6 +43,24 @@ end end + context "when given a connection" do + let(:conn) { instance_spy(ConnectionPool) } + let(:reaper) { :ruby } + let(:reaper_spy) { instance_spy("SidekiqUniqueJobs::Orphans::Reaper") } + + before do + allow(reaper_spy).to receive(:call) + allow(described_class).to receive(:new).and_return(reaper_spy) + end + + it "calls the reaper with the given connection" do + call + + expect(reaper_spy).to have_received(:call) + expect(described_class).to have_received(:new).with(conn) + end + end + shared_examples "deletes orphans" do context "when scheduled" do let(:item) { raw_item.merge("at" => Time.now.to_f + 3_600) } @@ -109,36 +129,63 @@ end context "with job in process" do - let(:process_key) { "process-id" } - let(:thread_id) { "thread-id" } - let(:worker_key) { "#{process_key}:workers" } + let(:process_key) { "process-id" } + let(:thread_id) { "thread-id" } + let(:worker_key) { "#{process_key}:workers" } + let(:created_at) { (Time.now - reaper_timeout).to_f } + let(:reaper_timeout) { SidekiqUniqueJobs.config.reaper_timeout } before do SidekiqUniqueJobs.redis do |conn| conn.multi do conn.sadd("processes", process_key) conn.set(process_key, "bogus") - conn.hset(worker_key, thread_id, dump_json(payload: item)) + conn.hset(worker_key, thread_id, dump_json(created_at: created_at, payload: item)) conn.expire(process_key, 60) conn.expire(worker_key, 60) end end end - # TODO: Adjust this spec for earlier sidekiq versions context "that matches current digest", sidekiq_ver: ">= 5.0" do - it "keeps the digest" do - expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + context "and created_at is old" do # rubocop:disable RSpec/NestedGroups + let(:created_at) { (Time.now - (reaper_timeout + 100)).to_f } + + it "keeps the digest" do + expect { call }.not_to change { digests.count }.from(1) + expect(unique_keys).not_to match_array([]) + end + end + + context "and created_at is recent" do # rubocop:disable RSpec/NestedGroups + let(:created_at) { Time.now.to_f } + + it "keeps the digest" do + expect { call }.not_to change { digests.count }.from(1) + expect(unique_keys).not_to match_array([]) + end end end context "that does not match current digest" do let(:item) { { "class" => MyUniqueJob, "args" => [], "jid" => job_id, "lock_digest" => "uniquejobs:d2" } } - it "deletes the digest" do - expect { call }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + context "and created_at is old" do # rubocop:disable RSpec/NestedGroups + let(:created_at) { (Time.now - (reaper_timeout + 100)).to_f } + + it "deletes the digest" do + expect { call }.to change { digests.count }.by(-1) + expect(unique_keys).to match_array([]) + end + end + + context "and created_at is recent" do # rubocop:disable RSpec/NestedGroups + let(:created_at) { Time.now.to_f } + + it "keeps the digest" do + expect { call }.not_to change { digests.count }.from(1) + expect(unique_keys).not_to match_array([]) + end end end end diff --git a/spec/sidekiq_unique_jobs/redis/entity_spec.rb b/spec/sidekiq_unique_jobs/redis/entity_spec.rb index dadb68a50..5730c6a16 100644 --- a/spec/sidekiq_unique_jobs/redis/entity_spec.rb +++ b/spec/sidekiq_unique_jobs/redis/entity_spec.rb @@ -6,4 +6,20 @@ let(:key) { "digest" } its(:count) { is_expected.to eq(0) } + + describe "#exist?" do + subject(:exist?) { entity.exist? } + + context "when key exists" do + before do + set(key, "bogus") + end + + it { is_expected.to eq(true) } + end + + context "when key does not exist" do + it { is_expected.to eq(false) } + end + end end diff --git a/spec/sidekiq_unique_jobs/web_spec.rb b/spec/sidekiq_unique_jobs/web_spec.rb index e0b16f7b7..d61557884 100644 --- a/spec/sidekiq_unique_jobs/web_spec.rb +++ b/spec/sidekiq_unique_jobs/web_spec.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "sidekiq/web" require "sidekiq_unique_jobs/web" require "rack/test" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4da3036b1..9c786cb1c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,7 +3,7 @@ require "bundler/setup" if RUBY_ENGINE == "ruby" && RUBY_VERSION >= "2.6" - require "simplecov" unless %w[false 0].include?(ENV["COV"]) + require "simplecov" if ENV["COV"] begin require "pry" @@ -43,10 +43,12 @@ config.define_derived_metadata do |meta| meta[:aggregate_failures] = true end + config.expect_with :rspec do |expectations| expectations.include_chain_clauses_in_custom_matcher_descriptions = true end config.mock_with :rspec do |mocks| + mocks.allow_message_expectations_on_nil = true mocks.verify_partial_doubles = true end config.example_status_persistence_file_path = ".rspec_status"