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

Add support for Backing off at Stream Level #129

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

bartelink
Copy link
Collaborator

@bartelink bartelink commented Nov 30, 2021

Follow-on from #126 adding support for backoffs. Such backoffs might be useful for the following scenarios:

  • if a watchdog is monitoring a stream, it can have a quick look at any time, but it ca't and shouldn't do anything useful until the timeout has been reached. In this case, it can pass an Outcome of BackOffUntil time and the HandleOk can set it (except this PR does not pass the stream name to the success handler)
  • In general, the Cosmos SDK's intrinsic backoffs work ok and add some backpressure (at the cost of occupying a thread), which avoids the next handler invocation immediately also hitting the same issue. The mechanism here of backing off on one stream is as likely to be counterproductive (generating and sending another failing request right away) as productive (switch to another stream on another node that's not yet hit the rate limit threshold this time)
  • others? I'd like to see real scenarios so the design can actually be validated. While this code is low risk and relatively efficient, I really don't want to make people read it and/or deal with breaking changes and/or maintaining it unless it's actually proven to solve a legitimate need in the best way possible

@bartelink
Copy link
Collaborator Author

@ragiano215 This is what popped out - it has a breaking change and I am not personally using it yet so I might hold off on merging

@bartelink bartelink changed the title Add backoff impl Add support for Backing off at Stream Level Nov 30, 2021
Base automatically changed from stuck to master December 6, 2021 08:58
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.

1 participant