Only toggle production state when above/below watermark #2996
Conversation
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
Motivation: The high/low watermark backpressure strategy assumes that a yield can never happen when there's no outstanding demand. It's reasonably easy for this to not be a true: a handler decoding messages in a loop, for example, may not pay attention to `read() calls and produce when there's no outstanding demand. The channel should of course not read from the socket, so produced values will stop being produced. Modifications: - Alter the high/low watermarks such that demand is only enabled when the buffer depth is below the low watermark, and only disabled when above the high watermark. Result: Fewer crashes
FranzBusch
approved these changes
Nov 25, 2024
glbrntt
added a commit
to glbrntt/swift-nio
that referenced
this pull request
Nov 25, 2024
Motivation: The high/low watermark backpressure strategy assumes that a yield can never happen when there's no outstanding demand. It's reasonably easy for this to not be a true: a handler decoding messages in a loop, for example, may not pay attention to `read() calls and produce when there's no outstanding demand. The channel should of course not read from the socket, so produced values will stop being produced. Modifications: - Alter the high/low watermarks such that demand is only enabled when the buffer depth is below the low watermark, and only disabled when above the high watermark. Result: Fewer crashes (cherry picked from commit ba6608e)
glbrntt
added a commit
that referenced
this pull request
Nov 25, 2024
Motivation: The high/low watermark backpressure strategy assumes that a yield can never happen when there's no outstanding demand. It's reasonably easy for this to not be a true: a handler decoding messages in a loop, for example, may not pay attention to `read() calls and produce when there's no outstanding demand. The channel should of course not read from the socket, so produced values will stop being produced. Modifications: - Alter the high/low watermarks such that demand is only enabled when the buffer depth is below the low watermark, and only disabled when above the high watermark. Result: Fewer crashes (cherry picked from commit ba6608e)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation:
The high/low watermark backpressure strategy assumes that a yield can never happen when there's no outstanding demand. It's reasonably easy for this to not be a true: a handler decoding messages in a loop, for example, may not pay attention to `read() calls and produce when there's no outstanding demand. The channel should of course not read from the socket, so produced values will stop being produced.
Modifications:
Result:
Fewer crashes