Backport #756 to v7.1 #789
Closed
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
I was going through the docs after having some trouble with this and noticed that DailyDrip is no longer a thing :( Using the direct link to Sidekiq's wiki instead.
* Fix edge case in ruby reaper Closes #653 The problem was that the reaper returns nil instead of the memo. Many thanks to @francesmcmullin for the wonderfully detailed report and investigation. Now we have a regression test for this behavior. * Mandatory rubocop commit
* Improve reaper performance under heavy load Closes #663 Signed-off-by: mhenrixon <mikael@mhenrixon.com> * Mandatory linter commit
The `mhenrixon.github.io` urls result into `404` Co-authored-by: Mikael Henriksson <mikael@mhenrixon.com>
* Bump bundler and friends * Fix ruby 3 specific test error? * Get rid of warnings against sidekiq/master * Skip failing test against ruby 3
* Prepare for redis 5.0.0 * Run on ruby 3.1 * Mandatory linter commit * Mandatory linter commit
* Fix the remaining deprecation warnings * Mandatory linter commit * Allow pipeline to be passed on
* `on_conflict { server: :reschedule }` always uses default queue.
I noticed, that when I use the `on_conflict` strategy `:reschedule` with a job, that has a
`until_and_while_executing` lock and is configured to run on a specific
queue,(e.g. `no_default`). `:reschedule` tries to enqueue it into the
`default` queue.
The commit lets `:reschedule` honour the queue.
* Modify specs to honour queue.
* Listen to Rubocop, wise he is.
As mentioned in README, to show ❤️ (support). https://guides.rubygems.org/specification-reference/
* Fix drift * Smells like a fish * Mandatory rubocop commit * Fix broken tests against master * Mandatory linter commit * Fix concurrent ruby warnings
Avoid negative ranges when looping through queues
Co-authored-by: Mikael Henriksson <mikael@mhenrixon.com>
* Fix Sidekiq::Worker.clear_all override not being applied Since the method is defined as a singleton the testing override needs to be prepended to the singleton class. * Rename `testing_spec` to `worker_spec` `
* Chore(deps): Bump bundler on Actions * Chore(deps): Adds toxiproxy * Chore(test): Attempt to break push * Chore(test): Account for CI * fix(toxiproxy): replicate the problem * Chore(docker): open ports to toxiproxy? * Fix(timouts): Always wait a minimal amount * Chore(toxiproxy): Shopify isn't putting much effort into ubuntu * Chore(smell): Clean up * Chore(lint): Mandatory linter commit * Chore(CI): Run RSpec RUUUUNN! * Chore(CI): Skip on CI because... toxiproxy... * Fix(locksmith): Allow immediate nonblock pop
* Fix(unlock): Delete primed keys on last entry * Chore(test): Add test coverage
…#732) * Chore(deps): Add redis-namespace for debugging * Fix(namespace): Ensure compatible with namespaces * Chore(lint): Address code smells
* fix: consistent digests * chore(lint): lint'em real good * fix(redis): silence deprecations * fix(spec): another wrong digest * chore(deps): ignore sidekiq develop for now * chore(deps): stupid compatibility * fix(redis): use the ruby driver * chore(deps): lock redis to < 5.0
BACKGROUND ------------------------------------------------------------- #707 introduced a regression where the instance level `after_unlock` callback is no longer being called. This was due to the change of using `item[CLASS]` instead of `worker_class` to set `job_class` in `SidekiqUniqueJobs::Middelware#call`. `item[CLASS]` is a string, where as `worker_class` can be either an instance, or the class itself. For the instance level `after_unlock` to work, we need `job_class` to be an instance of the worker. This fixes the issue by switching back to setting `job_class` to `worker_class` CHANGELOG -------------------------------------------------------------- - Use worker_class for self.job_class in Middleware#call Co-authored-by: Mikael Henriksson <mikael@mhenrixon.com>
|
edit: scratch my prior message, now I see what you're describing: the v7.1.x releases were tagged from somewhere else and are not reflected on the v7.1 branch. So maybe there's a two-step change required here: first @mhenrixon if you could merge the 7.1.29 tag to the v7.1 branch, then @andrepiske a PR that cherry-picks #756 onto the updated v7.1 branch. @mhenrixon then you could tag release version v7.1.30 on the v7.1 branch and all is good. Super appreciate finding this gem that looks like it will solve exactly the locking/rescheduling need I'm running in to! |
|
This should be released as v7.1.30 https://github.com/mhenrixon/sidekiq-unique-jobs/releases/tag/v7.1.30 |
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.
Note: I opened this branch from the
v7.1.29tag, but I'm trying to merge it into thev7.1branch, which seems outdated (last commit Oct 2021 vs Dec 2022 for the tag). My change is only 1 commit long. I guess the v7.1 branch has to be updated with the v7.1.29 tag, as I can't open a PR to a tag. Or perhaps I'm doing something wrong here :)I'm having exactly the same problem as #779 and this PR solves it by backporting PR #756, which applies to v8 of this gem, to v7.
I couldn't find any other instances of the string
:workersthat should be changed and I also tested this on my dev machine with a real app and it fixes the problem.Let me know if this looks good enough!