Skip to content

outbuf_high_watermark#242

Merged
digitalresistor merged 6 commits intomasterfrom
outbuf-high-watermark
Apr 6, 2019
Merged

outbuf_high_watermark#242
digitalresistor merged 6 commits intomasterfrom
outbuf-high-watermark

Conversation

@mmerickel
Copy link
Member

@mmerickel mmerickel commented Apr 2, 2019

Fixes #67.

The outbuf_high_watermark is a setting that can be used to apply backpressure on the app_iter if it is yielding data faster than can be written to the client. When the socket falls behind, the app_iter will not resume until the socket catches up to at least the watermark (this does not mean the socket is caught up, just that it's not so far behind).

@mmerickel mmerickel force-pushed the outbuf-high-watermark branch from 0329dab to b9dbed8 Compare April 2, 2019 17:18
@mmerickel mmerickel force-pushed the outbuf-high-watermark branch 2 times, most recently from ba55660 to 908573f Compare April 4, 2019 07:12
@mmerickel mmerickel marked this pull request as ready for review April 4, 2019 07:12
@mmerickel mmerickel force-pushed the outbuf-high-watermark branch 2 times, most recently from 5722d8e to 6534d80 Compare April 4, 2019 18:51
@mmerickel mmerickel force-pushed the outbuf-high-watermark branch from 6534d80 to 7268909 Compare April 5, 2019 18:01
@mmerickel mmerickel force-pushed the outbuf-high-watermark branch from 7268909 to b4df005 Compare April 5, 2019 18:08
digitalresistor and others added 2 commits April 5, 2019 13:17
Co-Authored-By: mmerickel <github@m.merickel.org>
@mmerickel mmerickel force-pushed the outbuf-high-watermark branch from b9d0a3e to 291f8e1 Compare April 5, 2019 18:37
Copy link
Member

@digitalresistor digitalresistor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, minor nitpicks.

# that we need to account for, otherwise it'd be better
# to do this check at the start of the request instead of
# at the end to account for consecutive service() calls
if len(self.requests) > 1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense here to kick the trigger?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the scenarios in which kicking the trigger is required.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not like the current behavior is worse than it was before. I would assume the situation is that the write returned 0 bytes written because the buffer was full, and the system would know that more data is writable before hitting a select timeout or something. If you think it's a problem then it should be kicked in _flush_outbufs_below_high_watermark which affects both app_iter writes as well as here between requests. I can't imagine the circumstances are different here than there.

@digitalresistor digitalresistor merged commit 5583715 into master Apr 6, 2019
@digitalresistor digitalresistor deleted the outbuf-high-watermark branch April 6, 2019 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants