AppServerMetrics reporting-thread fix #24967
Merged
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.
This PR resolves an issue with the
AppServerMetrics
metrics-reporting thread being shutdown by Puma, by manually spawning the reporting task in abefore_block
instead of implicitly when the middleware is instantiated.The issue stems from a slight apparent difference between Puma and Unicorn in the daemonization process. While Unicorn preserves threads created by the master process, Puma shuts down any threads spawned during the app-preload process unless they are created after
daemonize!
occurs within thebefore_fork
hook.In order to access the middleware instance within the hook, I added an
instance
class-accessor to the middleware class. This doesn't seem ideal, but I couldn't figure any better way to access the middleware instance (since the instance is created dynamically from the Rack builder when the middleware stack is built on startup, there's no place in code we can assign the individual instance for later access). This shouldn't be a problem in practice, since there should only be a single instance of this middleware-class created per process.