-
Notifications
You must be signed in to change notification settings - Fork 396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Run consume logic in a separate thread #982
Merged
GaryWilber
merged 21 commits into
Blizzard:master
from
GaryWilber:user/garywilb/threaded_consume_3
Jan 20, 2023
Merged
Run consume logic in a separate thread #982
GaryWilber
merged 21 commits into
Blizzard:master
from
GaryWilber:user/garywilb/threaded_consume_3
Jan 20, 2023
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
Ping @webmakersteve , @iradul, @eandersson , @BlizzTom |
Hello, I am currently experiencing the same issue. Is it still planned to merge this PR? Thank you in advance and thank you for the work you are doing. Regards. |
sharmakhushboo
approved these changes
Jan 20, 2023
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.
The current consume method works like this:
That logic is not great. The while() loop is essentially blocking one of the 4 node.js worker threads from doing any other work because it took over with its infinite loop. You can confirm this yourself by starting an HTTP server & then calling the consume method. The HTTP server will not process any requests because it’s unable to schedule the work since that node.js worker thread is blocked – this repro’s 100% of the time for me on node.js 16 with a fastify webserver.
Some reading regarding node.js & it’s worker threads / thread pool to learn more:
This PR does the following:
These changes result in a 30% latency reduction for my scenario. My application has been running with these changes for a while now and it has been stable.
This PR should be checked in as a squash commit due to the amount of commits.