Handle 429 from accountant - #2101
Merged
Merged
Conversation
vkuznecovas
requested review from
Waldz,
anjmao,
soffokl,
tadaskay and
zolia
as code owners
April 22, 2020 14:49
anjmao
reviewed
Apr 22, 2020
| return res, backoff.Retry(func() error { | ||
| err = ac.doRequest(req, &res) | ||
| if err != nil { | ||
| // if too many requests, retry |
Contributor
There was a problem hiding this comment.
Are we sure that after 1.5 second lock will be released? Maybe it would be safer and more clear to block in endpoint so it waits until lock is freed inside hermes (the same as simple locks works). In case it can't finish (let's say lock is not freed for some reason) this request will timeout.
Contributor
Author
There was a problem hiding this comment.
This is indented to TRY again, not to ensure success. The accountant will lock for very short periods of time(less than 300ms currently). Besides, this error is not critical at all, as you can skip a few reveals/requests just fine.
tadaskay
approved these changes
Apr 23, 2020
anjmao
approved these changes
Apr 23, 2020
vkuznecovas
force-pushed
the
accountant-429
branch
from
April 23, 2020 06:57
394e035 to
4ae74de
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Accountant will now return 429 on request promise and reveal R.
This PR handles them in the following way:
Accountant caller will retry a request promise and reveal R request if a 429 is encountered. Upon encountering a non 429 error, the error will be bubbled straight away.