Move WorkerHealthChecker from app/services to lib #1689
Merged
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
|
Looks like we need to add a missing Also should we move the corresponding spec from |
**Why**: Middleware, such as `WorkerHealthChecker::Middleware` can't be reloaded, but because the `WorkerHealthChecker` module was placed in `app/services`, it automatically gets reloaded by Rails, which results in the following error when running Sidekiq jobs while Rails is trying to clean up and unload code: ``` ArgumentError: A copy of WorkerHealthChecker::Middleware has been removed from the module tree but is still active! ``` The solution is to move the module to a path that is not included in the Rails autoload paths, such as `lib`. A negative side effect of this issue is that any background job that ran before Rails raised the error (when it was trying to clean up and unload the code), will keep running over and over because Sidekiq automatically retries failed jobs. I experienced this locally by getting the same email over and over, and I'm assuming this is the same issue that caused some of our production users to keep getting the same SMS over and over. Jenny also saw this in INT in June.
monfresh
force-pushed
the
mb-fix-sidekiq-autoload-error
branch
from
17621cc to
89d6193
Compare
|
Good catches. Fixed! |
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.
Why: Middleware, such as
WorkerHealthChecker::Middlewarecan't bereloaded, but because the
WorkerHealthCheckermodule was placed inapp/services, it automatically gets reloaded by Rails, which resultsin the following error when running Sidekiq jobs while Rails is trying
to clean up and unload code:
The solution is to move the module to a path that is not included in
the Rails autoload paths, such as
lib.A negative side effect of this issue is that any background job that
ran before Rails raised the error (when it was trying to clean up and
unload the code), will keep running over and over because Sidekiq
automatically retries failed jobs. I experienced this locally by
getting the same email over and over, and I'm assuming this is the
same issue that caused some of our production users to keep getting
the same SMS over and over. Jenny also saw this in INT in June.