Fix/verify rate limits #81
Conversation
|
nice change! should there be tests for this code segment? |
|
for |
|
/lgtm |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
| @@ -140,6 +140,7 @@ func (b *ClientBuilder) Build() (Client, error) { | |||
| MaxRetries: aws.Int(25), | |||
| // Set MinThrottleDelay to 1 second | |||
| Retryer: client.DefaultRetryer{ | |||
| NumMaxRetries: 5, | |||
There was a problem hiding this comment.
I could have been sure that I added this initially! Thanks. Though the hack day testing we found that retrying didn't always resolve the issue its annoying how inconsistent this is. Did you try with another AWS account?
There was a problem hiding this comment.
I've tested it with 2 different accounts. One is my personal and the other what @fahlmant gave me. The retry is not a permanent solution I agree, but still, the number of 400s significantly reduced(I didn't see in the logs any but still can occur..). Having said that, there's no response code that resp contains (resp, err := client.iamClient.SimulatePrincipalPolicy(input)) It only fills the err message with status code when anything happens except 200. Because of that, it's hard to adjust the retry numbers dynamically if continuously 400 happens
There was a problem hiding this comment.
what could be the other alternatives? We're making a single request for policy checks so I don't think we can reduce the number of requests at all.
There was a problem hiding this comment.
Ok no problem, lets see how thing goes and look at it closer if we continue to see rate limiting errors.
|
/unhold |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/lgtm |
|
@jharrington22 - you had a change request earlier, does this lgty now? |
Fixes: OSD-4671
I've realized a
Retryer: client.DefaultRetryerwas defined butNumMaxRetrieswas missing. By default, the value is0which means there will be no retry unless we increase the number.Results are looking good, I've tested it in while loop on 5 different tabs, didn't see any
400but responses may take longer as a result of retry mechanism.