Allow maxCASAttempts to be configurable

[roshan-gorasia-cko]

Hi,

We've recently come in need of having a configurable retry limit (maxCASAttemptsLimit). Currently as is, this is a constant set to 10 - Which isn't flexible in certain use cases.

This PR adds a NewGCRARateLimiterWithRetryLimit() function which can take in a user defined retry limit. The current NewGCRARateLimiter() in place will make use of the existing maxCASAttemptsLimit of 10.

While here, I also noticed that the current logic was always retrying +1 times of the defined limit. Therefore it was always attempting a maximum of 11 times with the default max attempt limit of 10. I've addressed this so the limit comparison is greater than or equal to (as opposed to greater than only)

Let me know what you think

Relates to: #91

[brandur]

@roshan-gorasia-cko What do you think about making this a setter on the limiter like SetMaxCASAttemptsLimit instead of a new constructor? I suspect that this option won't be needed all too often so it probably makes more sense to keep a little more hidden away.

[roshan-gorasia-cko]

@roshan-gorasia-cko What do you think about making this a setter on the limiter like SetMaxCASAttemptsLimit instead of a new constructor? I suspect that this option won't be needed all too often so it probably makes more sense to keep a little more hidden away.

@brandur Agreed! I've committed the changes

Just for my knowledge, what is the reasoning behind such an (arguably) high retry limit?

[brandur]

Thanks Roshan! Sorry about taking so long to get back on this. Changes LGTM.

Just for my knowledge, what is the reasoning behind such an (arguably) high retry limit?

So I'm not the original author (mostly just maintain it), but I bet the decision was pretty arbitrary — 10 is a nice, round number and they probably just went with that.

I think you're right though in that this isn't an optimal choice. If you are ever getting anywhere near 10 attempts on a regular basis than something is almost certainly wrong in that you're seeing way too much contention in the system. It could still work for a while, but in practice it'd mean that all rate limiting operations are taking way longer than they're supposed to (maybe 10x as long!). It would probably be smarter to only retry a few times and then error out earlier.

[brandur]

Released as version 2.9.0.