Skip to content
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

Restore RequestResponse::throttled. #1715

Closed
wants to merge 5 commits into from

Conversation

twittner
Copy link
Contributor

Fixes #1706.
Continuation of #1714.

@twittner twittner force-pushed the throttled-fix branch 3 times, most recently from 77ddfb3 to 94f5671 Compare August 19, 2020 14:58
@twittner twittner marked this pull request as ready for review August 19, 2020 15:44
@twittner twittner marked this pull request as draft August 19, 2020 16:07
@twittner twittner force-pushed the throttled-fix branch 2 times, most recently from 87a0f25 to fc692f1 Compare August 20, 2020 09:39
@twittner twittner marked this pull request as ready for review August 20, 2020 11:33
@romanb
Copy link
Contributor

romanb commented Aug 24, 2020

Would you mind rebasing or merging master for ease of review, since #1714 is now merged?

Fix the budget calculation using `ProtocolsHandler::InboundOpenInfo`.
Create events when connection closed.
romanb
romanb previously approved these changes Aug 24, 2020
protocols/request-response/src/throttled.rs Outdated Show resolved Hide resolved
@twittner
Copy link
Contributor Author

Thinking a little more about the approach here I think we have a race condition w.r.t. timeouts and errors. If the sender of a request times out or errors earlier than the receiver it grants itself another request. Should that request reach the receiver before it has detected the timeout or error and likewise allowed another request from the sender, it would potentially generate a TooManyInboundRequests event.

Without a way to give credit to a remote that is independent of receiving a request, i.e. being able to send unidirectional and unsolicited credit grants (which allows the receiver to retry sending the credit after a timeout or error) there is always a need for both ends to adjust their credit views which leads to inconsistencies as described above.

@romanb
Copy link
Contributor

romanb commented Sep 7, 2020

Ultimately superseded by #1726.

@romanb romanb closed this Sep 7, 2020
@twittner twittner deleted the throttled-fix branch September 7, 2020 15:29
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.

request-response: Throttled can not count its receive budget properly.
2 participants