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

feat(digest): allow modern algorithm #853

Merged
merged 1 commit into from
Jul 25, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/rspec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ jobs:
- sidekiq_7.0
- sidekiq_7.1
- sidekiq_7.2
- sidekiq_7.3

steps:
- uses: actions/checkout@v4
Expand Down
22 changes: 21 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,24 @@ AllCops:
- "myapp/**/*"
- "Sidekiq/**/*"
- "vendor/**/*"
Layout/ArgumentAlignment:
Enabled: true
EnforcedStyle: with_fixed_indentation

Layout/EndAlignment:
EnforcedStyleAlignWith: variable

Layout/FirstArrayElementIndentation:
EnforcedStyle: consistent

Layout/FirstHashElementIndentation:
EnforcedStyle: consistent

Layout/HashAlignment:
EnforcedColonStyle: key
EnforcedHashRocketStyle: key
EnforcedLastArgumentHashStyle: always_inspect

Layout/LineContinuationLeadingSpace:
Enabled: false

Expand Down Expand Up @@ -115,7 +129,13 @@ RSpec/ExpectActual:
RSpec/ExpectChange:
EnforcedStyle: block

RSpec/FilePath:
RSpec/SpecFilePathFormat:
Enabled: true
Exclude:
- spec/performance/locksmith_spec.rb
- spec/performance/lock_digest_spec.rb

RSpec/SpecFilePathSuffix:
Enabled: true
Exclude:
- spec/performance/locksmith_spec.rb
Expand Down
4 changes: 4 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,7 @@ end
appraise "sidekiq-7.2" do
gem "sidekiq", "~> 7.2.0"
end

appraise "sidekiq-7.3" do
gem "sidekiq", "~> 7.3.0"
end
35 changes: 24 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Want to show me some ❤️ for the hard work I do on this gem? You can use the
- [sidekiq-global_id](#sidekiq-global_id)
- [sidekiq-status](#sidekiq-status)
- [Global Configuration](#global-configuration)
- [digest_algorithm](#digest_algorithm)
- [debug_lua](#debug_lua)
- [lock_timeout](#lock_timeout)
- [lock_ttl](#lock_ttl)
Expand Down Expand Up @@ -734,17 +735,29 @@ Configure SidekiqUniqueJobs in an initializer or the sidekiq initializer on appl

```ruby
SidekiqUniqueJobs.configure do |config|
config.logger = Sidekiq.logger # default, change at your own discretion
config.logger_enabled = true # default, disable for test environments
config.debug_lua = false # Turn on when debugging
config.lock_info = false # Turn on when debugging
config.lock_ttl = 600 # Expire locks after 10 minutes
config.lock_timeout = nil # turn off lock timeout
config.max_history = 0 # Turn on when debugging
config.reaper = :ruby # :ruby, :lua or :none/nil
config.reaper_count = 1000 # Stop reaping after this many keys
config.reaper_interval = 600 # Reap orphans every 10 minutes
config.reaper_timeout = 150 # Timeout reaper after 2.5 minutes
config.logger = Sidekiq.logger # default, change at your own discretion
config.logger_enabled = true # default, disable for test environments
config.debug_lua = false # Turn on when debugging
config.lock_info = false # Turn on when debugging
config.lock_ttl = 600 # Expire locks after 10 minutes
config.lock_timeout = nil # turn off lock timeout
config.max_history = 0 # Turn on when debugging
config.reaper = :ruby # :ruby, :lua or :none/nil
config.reaper_count = 1000 # Stop reaping after this many keys
config.reaper_interval = 600 # Reap orphans every 10 minutes
config.reaper_timeout = 150 # Timeout reaper after 2.5 minutes
config.digest_algorithm = :modern # Timeout reaper after 2.5 minutes
Copy link

Choose a reason for hiding this comment

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

The comment is wrong here and describes reaper_timeout

end
```
#### digest_algorithm

For backwards compatibility this one is set to `:legacy` by the default. If you happen to run into issues with FIPS being enabled on your redis server you might want to set this to `:modern`.

See: https://github.com/mhenrixon/sidekiq-unique-jobs/issues/848 for explanation

```ruby
SidekiqUniqueJobs.configure do |config|
config.digest_algorithm = :modern # Timeout reaper after 2.5 minutes
end
```

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/sidekiq_7.1.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ gem "sinatra"
gem "timecop"
gem "toxiproxy"
gem "yard"
gem "sidekiq", "~> 7.0.0"
gem "sidekiq", "~> 7.1.0"

platforms :mri do
gem "concurrent-ruby-ext"
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/sidekiq_7.2.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ gem "sinatra"
gem "timecop"
gem "toxiproxy"
gem "yard"
gem "sidekiq", "~> 7.0.0"
gem "sidekiq", "~> 7.2.0"

platforms :mri do
gem "concurrent-ruby-ext"
Expand Down
28 changes: 28 additions & 0 deletions gemfiles/sidekiq_7.3.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "appraisal"
gem "faraday-retry"
gem "gem-release"
gem "github-markup"
gem "rack-test"
gem "rake", "13.0.3"
gem "reek", ">= 5.3"
gem "rspec"
gem "rspec-benchmark"
gem "rspec-html-matchers"
gem "rspec-its"
gem "rubocop-mhenrixon"
gem "simplecov-sublime", ">= 0.21.2", require: false
gem "sinatra"
gem "timecop"
gem "toxiproxy"
gem "yard"
gem "sidekiq", "~> 7.3.0"

platforms :mri do
gem "concurrent-ruby-ext"
end

gemspec path: "../"
80 changes: 50 additions & 30 deletions lib/sidekiq_unique_jobs/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,29 @@

module SidekiqUniqueJobs
# ThreadSafe config exists to be able to document the config class without errors
ThreadSafeConfig = Concurrent::MutableStruct.new("ThreadSafeConfig",
:lock_timeout,
:lock_ttl,
:enabled,
:lock_prefix,
:logger,
:logger_enabled,
:locks,
:strategies,
:debug_lua,
:max_history,
:reaper,
:reaper_count,
:reaper_interval,
:reaper_timeout,
:reaper_resurrector_interval,
:reaper_resurrector_enabled,
:lock_info,
:raise_on_config_error,
:current_redis_version)
ThreadSafeConfig = Concurrent::MutableStruct.new(
"ThreadSafeConfig",
:lock_timeout,
:lock_ttl,
:enabled,
:lock_prefix,
:logger,
:logger_enabled,
:locks,
:strategies,
:debug_lua,
:max_history,
:reaper,
:reaper_count,
:reaper_interval,
:reaper_timeout,
:reaper_resurrector_interval,
:reaper_resurrector_enabled,
:lock_info,
:raise_on_config_error,
:current_redis_version,
:digest_algorithm,
)

#
# Shared class for dealing with gem configuration
Expand Down Expand Up @@ -118,11 +121,9 @@ class Config < ThreadSafeConfig
#
# @return [3600] check if reaper is dead each 3600 seconds
REAPER_RESURRECTOR_INTERVAL = 3600

#
# @return [false] enable reaper resurrector
REAPER_RESURRECTOR_ENABLED = false

#
# @return [false] while useful it also adds overhead so disable lock_info by default
USE_LOCK_INFO = false
Expand All @@ -132,6 +133,9 @@ class Config < ThreadSafeConfig
#
# @return [0.0.0] default redis version is only to avoid NoMethodError on nil
REDIS_VERSION = "0.0.0"
#
# @return [:legacy] default digest algorithm :modern or :legacy
DIGEST_ALGORITHM = :legacy

#
# Returns a default configuration
Expand Down Expand Up @@ -198,6 +202,7 @@ def self.default # rubocop:disable Metrics/MethodLength
USE_LOCK_INFO,
RAISE_ON_CONFIG_ERROR,
REDIS_VERSION,
DIGEST_ALGORITHM,
)
end

Expand All @@ -210,8 +215,8 @@ def self.default # rubocop:disable Metrics/MethodLength
# @return [<type>] <description>
#
def default_lock_ttl=(obj)
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
" Please use `#{class_name}#lock_ttl=` instead."
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
"Please use `#{class_name}#lock_ttl=` instead."
self.lock_ttl = obj
end

Expand All @@ -224,8 +229,8 @@ def default_lock_ttl=(obj)
# @return [Integer]
#
def default_lock_timeout=(obj)
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
" Please use `#{class_name}#lock_timeout=` instead."
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
"Please use `#{class_name}#lock_timeout=` instead."
self.lock_timeout = obj
end

Expand All @@ -236,8 +241,8 @@ def default_lock_timeout=(obj)
# @return [nil, Integer] configured value or nil
#
def default_lock_ttl
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
" Please use `#{class_name}#lock_ttl` instead."
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
"Please use `#{class_name}#lock_ttl` instead."
lock_ttl
end

Expand All @@ -249,8 +254,8 @@ def default_lock_ttl
# @return [nil, Integer] configured value or nil
#
def default_lock_timeout
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated." \
" Please use `#{class_name}#lock_timeout` instead."
warn "[DEPRECATION] `#{class_name}##{__method__}` is deprecated. " \
"Please use `#{class_name}#lock_timeout` instead."
lock_timeout
end

Expand Down Expand Up @@ -304,6 +309,21 @@ def add_strategy(name, klass)
self.strategies = new_strategies
end

#
# Sets digest_algorithm to either :modern or :legacy
#
# @param [Symbol] value
#
# @return [Symbol] the new value
#
def digest_algorithm=(value)
unless [:modern, :legacy].include?(value)
raise ArgumentError, "Invalid digest algorithm: #{value} (should be :modern or :legacy)"
end

super
end

#
# The current version of redis
#
Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/digests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Digests < Redis::SortedSet
EMPTY_KEYS_SEGMENT = ["", "", "", ""].freeze

def initialize(digests_key = DIGESTS)
super(digests_key)
super
end

#
Expand Down
4 changes: 2 additions & 2 deletions lib/sidekiq_unique_jobs/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ def initialize(options)

super(
"#{job_class}##{lock_args_method} takes #{num_args} arguments, received #{given.inspect}" \
"\n\n" \
" #{source_location.join(':')}"
"\n\n " \
"#{source_location.join(':')}"
)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/lock/base_lock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def strategy_for(origin)
server_strategy
else
raise SidekiqUniqueJobs::InvalidArgument,
"#origin needs to be either `:server` or `:client`"
"#origin needs to be either `:server` or `:client`"
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq_unique_jobs/lock/while_executing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class WhileExecuting < BaseLock
# @param [Sidekiq::RedisConnection, ConnectionPool] redis_pool the redis connection
#
def initialize(item, callback, redis_pool = nil)
super(item, callback, redis_pool)
super
append_unique_key_suffix
end

Expand Down
6 changes: 3 additions & 3 deletions lib/sidekiq_unique_jobs/lock_args.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ def filter_by_symbol(args)
job_class.send(lock_args_method, args)
rescue ArgumentError
raise SidekiqUniqueJobs::InvalidUniqueArguments,
given: args,
job_class: job_class,
lock_args_method: lock_args_method
given: args,
job_class: job_class,
lock_args_method: lock_args_method
end

# The method to use for filtering unique arguments
Expand Down
7 changes: 6 additions & 1 deletion lib/sidekiq_unique_jobs/lock_digest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ def lock_digest
# Creates a namespaced unique digest based on the {#digestable_hash} and the {#lock_prefix}
# @return [String] a unique digest
def create_digest
digest = OpenSSL::Digest::MD5.hexdigest(dump_json(digestable_hash.sort))
digest = if SidekiqUniqueJobs.config.digest_algorithm == :legacy
OpenSSL::Digest::MD5.hexdigest(dump_json(digestable_hash.sort))
else
OpenSSL::Digest.new("SHA3-256", dump_json(digestable_hash.sort)).hexdigest
end

"#{lock_prefix}:#{digest}"
end

Expand Down
8 changes: 4 additions & 4 deletions lib/sidekiq_unique_jobs/locksmith.rb
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,10 @@ def primed_async(conn, wait = nil, &block) # rubocop:disable Metrics/MethodLengt
concurrent_timeout = add_drift(timeout)

reflect(:debug, :timeouts, item,
timeouts: {
brpoplpush_timeout: brpoplpush_timeout,
concurrent_timeout: concurrent_timeout,
})
timeouts: {
brpoplpush_timeout: brpoplpush_timeout,
concurrent_timeout: concurrent_timeout,
})

# NOTE: When debugging, change .value to .value!
primed_jid = Concurrent::Promises
Expand Down
4 changes: 2 additions & 2 deletions lib/sidekiq_unique_jobs/logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ def no_sidekiq_context_method
end

def fake_logger_context(_context)
logger.warn "Don't know how to setup the logging context. Please open a feature request:" \
" https://github.com/mhenrixon/sidekiq-unique-jobs/issues/new?template=feature_request.md"
logger.warn "Don't know how to setup the logging context. Please open a feature request: " \
"https://github.com/mhenrixon/sidekiq-unique-jobs/issues/new?template=feature_request.md"

yield
end
Expand Down
4 changes: 2 additions & 2 deletions lib/sidekiq_unique_jobs/on_conflict.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def self.find_strategy(strategy)

strategies.fetch(strategy.to_sym) do
SidekiqUniqueJobs.logger.warn(
"No matching implementation for strategy: #{strategy}, returning OnConflict::NullStrategy." \
" Available strategies are (#{strategies.inspect})",
"No matching implementation for strategy: #{strategy}, returning OnConflict::NullStrategy. " \
"Available strategies are (#{strategies.inspect})",
)

OnConflict::NullStrategy
Expand Down
Loading
Loading