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

Calculating sleep duration for WaitAndRetry, based on the execution result; eg 429 headers #177

Closed
jmansar opened this issue Nov 11, 2016 · 8 comments
Milestone

Comments

@jmansar
Copy link

jmansar commented Nov 11, 2016

Hi,

I have a small enhancement idea.

Currently the sleepDurationProvider function in the RetryWithSleep policy doesn't have access to the result of the executed action. The only value passed to the sleepDurationProvider is the retry count.

I have a scenario where it would be quite useful to know the result in the wait time calculation function.

Scenario

There is an API, which I call in my application. It returns the error response with the Retry-After HTTP header set, when client exceeds the request rate limit or it is temporarily unavailable.
The header contains number of seconds the client should wait before repeating the request.
It should be possible to use that value as a wait time.

Could you extend the sleepDurationProvider function to include the result from the executed action?

@reisenberger
Copy link
Member

@jmansar Thanks for bringing this really interesting idea across from our slack channel!

Main question in my mind: whether exposing result directly to sleepDurationProvider is the way we’d want to support this. Without knowing the underlying use case, some users might wonder: why is the execution result exposed to that policy-delegate? (and only that one?). And the pattern might not immediately make sense to extend to other policy-operation-influencing delegates.

So: how else can we do this, that keeps the Polly architecture evolving in a coherent way?

In this case I'm thinking we’d probably want to support this by expanding the role of the Context class in executions (something we intend anyway, when we move to support metrics). To fulfil what you need ... we would add the execution result instead to Context; and make sleepDurationProvider take Context as input param instead.

Making Context the central way we channel extra info to all parts of policy execution:

  • Follows a common pattern, with all the onStateChange delegates already taking Context
  • Reduces the overall number of overloads for different variants (already too proliferated)
  • Allows all the onStateChange delegates to also benefit from getting the execution result 😄

Note there are other similar (in a very broad sense) issues like #38 . Also about exchanging data between execution and Policy-operation-influencing delegates.

My suggestion-in-the-meantime for your use case is similar to at bottom of #38. Close over some common variable in the sleepDurationProvider and Execute delegates, to share that part of the response back to the sleepDurationProvider delegate

@johnmcase
Copy link

+1

Exact same use case for me. Passing in the exception as a parameter to the sleepDurationProvider would also work if that makes it simpler.

@reisenberger
Copy link
Member

@johnmcase @jmansar This is added to the roadmap, in the generalized/extensible way I propose we should take this forward.

@tcsatheesh
Copy link

+1
My scenario: When querying DocumentDb if the request rate is too high the error response from the server includes a RetryAfter which is the Timespan to wait before retrying. It would be helpful if I can use this as part of my Retry policy.

@reisenberger reisenberger changed the title Calculating sleep duration based on the execution result Calculating sleep duration for WaitAndRetry, based on the execution result Apr 8, 2017
@reisenberger
Copy link
Member

reisenberger commented Apr 19, 2017

@jmansar @johnmcase @tcsatheesh Planning to deliver this feature in the next release 😄 . (already coded)

Per my previous comment, Polly is being expanded so that every delegate referenced by a policy (executed delegates; and policy control delegates such as onRetry) can share the execution Context that travels with each execution. This generalised approach adds lots of flexibility to exchange data throughout a Polly execution, unlocking your feature and many other architecturally-similar requests, eg #73, #38, improves pattern for #122 .

For RetryAfter, you can share a RetryAfter value returned by an underlying system through the Context, and feed it into your sleepDurationProvider.

Code example:

EDIT: I plan to blog a detailed code example when released.

EDIT: The release may accompany the PolicyRegistry feature and interfaces, also in development.

@reisenberger reisenberger changed the title Calculating sleep duration for WaitAndRetry, based on the execution result Calculating sleep duration for WaitAndRetry, based on the execution result; eg 429 headers Apr 26, 2017
@reisenberger reisenberger added this to the v5.1 milestone May 2, 2017
@joelhulen
Copy link
Member

@jmansar This feature has been released in the v5.1 version of the package today. Thanks for the great suggestion, and thank you @dreisenberger again for all of your hard work to add this feature and context flowing in general!

@reisenberger
Copy link
Member

@jmansar @johnmcase @tcsatheesh Blogged the pattern for using this for RetryAfter: http://www.thepollyproject.org/2017/05/04/putting-the-context-into-polly/ . Shout if any questions/suggestions.

Intend to break out the blogged Use Cases also into separate How to sections of the wiki.

This also makes a great case study for building your own custom policy: intending to blog/wiki how to build a custom policy that wraps all this functionality round WaitAndRetry (rather than you having to write the Context stuff manually).

@reisenberger
Copy link
Member

@jmansar @johnmcase @tcsatheesh Polly v5.6.0 (uploading to nuget in next day or so) will include the ability for WaitAndRetry policies to define a sleepDurationProvider which can take the handled fault (exception; or returned result) as an input parameter to the sleepDurationProvider delegate.

This should make it even easier to use WaitAndRetry policies to wait for a 'retry after' duration specified by the called system; eg as CosmosDB does with HTTP response code 429 and response headers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants