Feature/fix on conflict handling #1
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dsander
force-pushed
the
feature/fix-on-conflict-handling
branch
2 times, most recently
from
d1762f9
to
e1610b5
Compare
* Necessary upgrades for Sidekiq v6.2.2 * Adds some coverage for the Sidekiq::Job scenario * Mandatory rubocop commit * Remove a number of gems I never user Also, install gems only when on Mac (prevents additional gems on CI) * Mandatory rubocop commit
dsander
force-pushed
the
feature/fix-on-conflict-handling
branch
from
e1610b5
to
f7cce0e
Compare
This fixes the exception posted below when using `on_conflict: replace` and potentially other bugs related to `on_conflict`. Before [this change][1] `BasicLock.lock` always returned either the acquired lock or the return value of `call_strategy`. Since `Middleware::Client#lock` now only yields when `lock_instance.lock` yields we have to also `yield` inside the lock instances when the lock was acquired via `lock_failed` (i.e. a `on_conflict` strategy). ``` NoMethodError: undefined method `key?' for "f5d69f8fd2e1f3dde8cee02e":String from sidekiq/client.rb:197:in `atomic_push' from sidekiq/client.rb:190:in `block (2 levels) in raw_push' from redis.rb:2489:in `block in multi' from redis.rb:69:in `block in synchronize' from monitor.rb:202:in `synchronize' from monitor.rb:202:in `mon_synchronize' from redis.rb:69:in `synchronize' from redis.rb:2483:in `multi' from sidekiq/client.rb:189:in `block in raw_push' from connection_pool.rb:63:in `block (2 levels) in with' from connection_pool.rb:62:in `handle_interrupt' from connection_pool.rb:62:in `block in with' from connection_pool.rb:59:in `handle_interrupt' from connection_pool.rb:59:in `with' from sidekiq/client.rb:188:in `raw_push' from sidekiq/client.rb:74:in `push' from sidekiq/worker.rb:240:in `client_push' from sidekiq/worker.rb:215:in `perform_in' ``` mhenrixon#590 [1]: s://github.com/mhenrixon/sidekiq-unique-jobs/commit/8c8d54c8b9dea363a7d8b8aeaceb2e82966b8503
When using the `on_conflict` `replace` strategy `base_lock` has to return the return value of the conflict strategy. The replace strategy removes the previous job from the scheduled/rety set or the queue. It is then re-queued inside `lock_failed`/`call_strategy`, thus we have to return the return value of which came from `lock` inside the block of ` client_strategy.call`.
dsander
force-pushed
the
feature/fix-on-conflict-handling
branch
from
31195e9
to
b3255e1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Debugging CI failures