-
-
Notifications
You must be signed in to change notification settings - Fork 4
fix: buffered channel consumed too much memory #106
Conversation
Signed-off-by: Valery Piashchynski <[email protected]>
WalkthroughThe code revision introduces enhancements to the worker pool's stream management, optimizing the flow of data through new channel mechanisms, fine-tuning buffer capacities, and refining the response processing logic. The changes aim to improve efficiency and stability, while also providing clearer logging for state changes within the system. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- go.mod
Files selected for processing (1)
- pool/static_pool/workers_pool.go (4 hunks)
Additional comments: 4
pool/static_pool/workers_pool.go (4)
241-247: The creation of the buffered channel
resp
for stream mode has been adjusted to a buffer size of 5. This change seems to be in line with the PR's objective to optimize memory usage. However, it's important to ensure that this new buffer size is sufficient for the expected load and does not introduce backpressure that could affect performance.285-287: The error handling within the stream mode seems to be improved by sending an error payload through the
resp
channel. This is a good practice as it allows the caller to handle the error appropriately. However, it's crucial to verify that all callers of this method properly handle the error payloads.293-293: Sending the payload through the
resp
channel in both supervised and non-supervised execution paths is consistent with the stream handling logic. It's important to ensure that the payloads are being consumed and that the channel is not getting blocked, which could lead to memory issues.Also applies to: 313-313
- 327-329: The creation of the buffered channel
resp
for req-resp mode with a buffer size of 1 is a significant change. This aligns with the PR's objective to reduce memory consumption. It's important to ensure that this change does not introduce any unexpected behavior or performance degradation.
Reason for This PR
ref: roadrunner-server/roadrunner#1830 (comment)
Description of Changes
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]
git commit -s
).CHANGELOG.md
.Summary by CodeRabbit