Delay initialization until Rails is fully initialized - #41
Open
DmitryTsepelev wants to merge 1 commit into
Open
Conversation
DmitryTsepelev
commented
Feb 1, 2021
| *NOTE:* As of 1.0.6 works with Sidekiq 4. | ||
| *NOTE:* As of 1.0.8 Locking is atomic (set nx/ex) and will no longer lead to batches that are permalocked and stuck | ||
|
|
||
| ## Installation |
Author
There was a problem hiding this comment.
Moved this section to the top because non-Rails users will have to perform an extra setup
houndci-bot
reviewed
Feb 1, 2021
DmitryTsepelev
commented
Feb 1, 2021
| end | ||
| end | ||
|
|
||
| Sidekiq::Grouping.start! if Sidekiq.server? |
Author
There was a problem hiding this comment.
This is a breaking change, so I guess we'll need a major release
DmitryTsepelev
force-pushed
the
railtie-after-config
branch
from
February 1, 2021 11:49
e3a1fbd to
b0a91ee
Compare
elcuervo
added a commit
to unsplash/sidekiq-grouping
that referenced
this pull request
Jul 10, 2023
Directly taken from gzigzigzeo#41 There's a potential issue with the initialization that prevents the config to be correctly loaded when in Rails. This enforces the right timing
|
Hi there are there any plans to merge this pull-request? |
|
We had a similar problem. In our case, the sidekiq-grouping initialization would clobber our redis URL initialization forcing the use of the default redis URL (localhost) which didn't work for our production setup. The workaround is to properly sequence the initialization. In rails, this can be done by: # config/initializers/sidekiq.rb
# The gem runs `Sidekiq::Grouping.start!` at require time (whenever Sidekiq.server? is true),
# spawning a background poller. Requiring it here, inside after_initialize, i.e. after all
# initializers, eager loading and Zeitwerk have finished, mirrors upstream PR #41
# (https://github.com/gzigzigzeo/sidekiq-grouping/pull/41), which is neither merged nor
# released. Deferring the poller this late guarantees Sidekiq's Redis URL is already set,
# so it builds the internal connection pool against the real Redis instead of
# memoizing the localhost fallback, and it avoids the poller autoloading app classes off
# the main thread before eager load.
Rails.application.config.after_initialize do
require 'sidekiq/grouping'
endAnd # Gemfile
gem 'sidekiq-grouping', require: false |
This file contains hidden or 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
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.
Related to the Zeitwerk issue about occasional
NameErrorinside SidekiqIn some cases sidekiq-grouping starts before Rails is fully initialized and Zeitwerk initial loading is completed, which can cause situations when some classes are loaded inside the concurrent thread rather than in Zeitwerk. This PR moves gem initialization to the Railtie.