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

handle 429 responses with exponential backoff retries #148

Open
alexkli opened this issue Feb 27, 2019 · 8 comments
Open

handle 429 responses with exponential backoff retries #148

alexkli opened this issue Feb 27, 2019 · 8 comments

Comments

@alexkli
Copy link
Contributor

alexkli commented Feb 27, 2019

When using openwhisk.action.invoke() or other operations one can encounter HTTP 429 TOO MANY REQUESTS status code responses from Openwhisk's rate limiting. It would be nice if the openwhisk js client can provide a retry mechanism out of the box.

The suggested approach is a exponential backoff. This was implemented e.g. here: apache/openwhisk-package-cloudant#90

@alexkli
Copy link
Contributor Author

alexkli commented Feb 27, 2019

Some thoughts on a possible implementation:

  • AFAICS, a retry would have to be added in Resources.operation() where it would apply to most operations (actions, triggers, ...).
  • Maybe it's necessary to limit it to certain operations only (GETs, invoke + fire) or maybe not, since it's a generic HTTP mechanism.
  • The API should allow to configure the timeouts, e.g. as part of the openwhisk constructor options.

I could provide a PR if that helps.

@starpit
Copy link
Contributor

starpit commented Feb 27, 2019

FYI prior discussion: #119

@alexkli
Copy link
Contributor Author

alexkli commented Feb 28, 2019

@starpit I saw it, but it was mentioning network level failures. I am quite explicitly interested in handling 429 rate limit errors, since we can get them consistently with too much traffic 😄

@jthomas
Copy link
Member

jthomas commented Feb 28, 2019

As discussed in #119, I didn't agree with introducing generic retry behaviour for all lower-level networking issues. However, I can see the case for a retry mechanism for explicit cases like the HTTP 429 returned by the platform API.

This should be turned off by default ("no surprises for the user and no breaking changes") and configurable through constructor options as @alexkli suggested. Exponential back-off is the correct approach.

@alexkli If you want to provide a PR - we can discuss the approach?

I was going to start the process of a release this library to push out the recent change (#147) but will wait until this is in?

Maybe it would be better to implement this in the client module rather than the resource class? Resources could pass a new parameter to indicate whether 429 errors with retryable. https://github.com/apache/incubator-openwhisk-client-js/blob/master/lib/client.js

@alexkli
Copy link
Contributor Author

alexkli commented Feb 28, 2019

I could take a look at it next week, but I wouldn’t want to block any release for it :-)

@alexkli
Copy link
Contributor Author

alexkli commented Mar 1, 2019

Right, in the client.js. I missed that this is where the http request logic is actually implemented.

@jthomas
Copy link
Member

jthomas commented Mar 12, 2019

Hello @alexkli - do you know if you'll be able to submit a PR this week? No rush - but as some other PRs have come in I'm now thinking about getting that release out.

@alexkli
Copy link
Contributor Author

alexkli commented Mar 15, 2019

@jthomas Unfortunately no, some other priorities pushed this down on my list. With the right concurrency settings in openwhisk the 429 problem went away for us (at least for now). Still planning to take a look maybe in a few weeks.

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

No branches or pull requests

3 participants