provider/azure: retry requests on status code 429 #5383

Merged
merged 3 commits into from May 12, 2016

Conversation

Projects
None yet
3 participants
Member

axw commented May 12, 2016

If Azure returns status code 429 (TooManyRequests), then that means we're
being rate-limited and we should back off and retry later. The 429
responses do not contain a Retry-After field, so we just use exponential
backoff starting at 5 seconds, going up to 1 minute between retries, with a
maximum of 5 minutes per API request.

Also updated Azure to the latest SDK.

Fixes https://bugs.launchpad.net/juju-core/+bug/1540394

(Review request: http://reviews.vapour.ws/r/4816/)

axw added some commits May 11, 2016

provider/azure: update Azure SDK
Update to the latest SDK which has some code
generation fixes and changes a few type names.

Bootstrapped and added a win2012 series machine.
provider/azure: retry requests on status code 429
If Azure returns status code 429 (TooManyRequests),
then that means we're being rate-limited and we
should back off and retry later. The 429 responses
do not contain a Retry-After field, so we just use
exponential backoff starting at 5 seconds, going up
to 1 minute between retries, with a maximum of
5 minutes per API request.

Fixes https://bugs.launchpad.net/juju-core/+bug/1540394
- )
- if err != nil {
+ var result storage.CheckNameAvailabilityResult
+ if err := callAPI(func() (autorest.Response, error) {
@wallyworld

wallyworld May 12, 2016

Owner

Should we define a type for this func signature

@axw

axw May 12, 2016

Member

Wouldn't help. You can't use a named function type in a function literal like that.

provider/azure/environ.go
@@ -836,14 +869,18 @@ func deleteInstance(
// Delete the VM's OS disk VHD.
logger.Debugf("- deleting OS VHD")
blobClient := storageClient.GetBlobService()
- if _, err := blobClient.DeleteBlobIfExists(osDiskVHDContainer, vmName); err != nil {
+ if err := callAPI(func() (autorest.Response, error) {
+ _, err := blobClient.DeleteBlobIfExists(osDiskVHDContainer, vmName)
@wallyworld

wallyworld May 12, 2016

Owner

Curious - why is this api not applicable to rate limiting?

@axw

axw May 12, 2016

Member

It's a different Azure API altogether. Actually there's no point that being wrapped in callAPI, I'll take it back out.

Owner

wallyworld commented May 12, 2016

LGTM

Owner

wallyworld commented May 12, 2016

$$fixes-1540394$$

Contributor

jujubot commented May 12, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 90ff01f into juju:master May 12, 2016

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