Skip to content

Conversation

odyhunter
Copy link

Add retry for wait_for_ready

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 85.049% when pulling f5115ac on odyhunter:delay_exponential into 05b95c7 on softlayer:master.

@allmightyspiff allmightyspiff self-assigned this Aug 31, 2017
@allmightyspiff
Copy link
Member

Thanks, I'll take a look at this tomorrow morning. There are a few changes I'll want to make as well, but I'll take care of that, so no worries.

Copy link

@gltdeveloper gltdeveloper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of point to consider...

  • AWS (I thought) uses exponential backoff (Math.pow). This does not. This may be a good first start into the test (to catch the initial set of failures/timeouts that may occur) - just something to consider. I do like the random nature given that our client will definitely have a large amount of concurrency.
  • In this implementation, it considers and does "x" retries where "x" is the configured # of retry attempts. One question we should ask ourselves is - for wait for ready, we are waiting for the VSI to be ready. If there are a number of concurrent requests all doing wait-for-ready, and we are near our API request limit per second, some requests may fail. So, do want to consider that if we fail on one request per timeout (we sleep/retry), and then are successful in issuing the request (but it's still not ready), so we reset the attempt counter? i.e., add attempt=1 to AFTER get_instance passes successfully? In this way, we tolerate more timeouts as long as progress is getting made on getting results..

@allmightyspiff
Copy link
Member

I'm closing this pull request in favor of #869

I'll make my comments about my choices in that pull request.

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

Successfully merging this pull request may close these issues.

4 participants