Auto-retry idempotent requests

[NautiluX]

When accessing OpenStack API with idempotent requests, my guess is that should apply to at least all GET or DELETE requests, they should be retried automatically in case of an error.
As an example take the request in list_ports.

It seems like excon already supports options to retry requests as the README states.

We face failures due to network issues repeatedly, which are not required to fail whole workflows without giving it a second try, hence this would be a very helpful improvement for us.

[gildub]

Sounds like a good idea!

I think we could offer the feature as an option, for instance as a global parameter.
So by default there are no options {} and if needed the option could be used on a per request basis or globally with {:idempotent => true}.

[NautiluX]

That sounds good! However, I think it would be even better for some of the API calls if the option was the default, as they are idempotent in OpenStack. That should for example apply to all the GET calls I think.

[gildub]

My concern is about the impact on existing deployments and more precisely is on what type of errors the :idempotent feature handles. Also what about the others requests PUT/POST, if you have network failures those are likely to fail too, isn't?

[NautiluX]

Yes. But in those cases, I think you cannot be sure the request you issued before didn't make it to OpenStack and hence you might, for example, create another disk if you retried the call. That's why those calls should fail. But, calls like GET or DELETE should be easily repeatable as issuing them multiple times should always result in the same state in OpenStack.

Regarding your concerns about existing deployments:
Maybe the global configuration could make it possible to enable idempotent flag for all calls of a given verb? (something like {:idempotent => ["GET", "DELETE"]})
Or the global configuration you mentioned could be an opt-in to all idempotent calls being retried, while the default stays to not retry them.

[gildub]

Thanks for the details, it makes sense that GET and DELETE are idempotent by nature vs the modifying verbs.

For the option we could combine both options {:idempotent => ["GET", "DELETE"]} and {:idempotent => true}. The first determines the VERBS being idempotent and the second activate the feature.

What do you think?

[NautiluX]

Sounds good. What would the default be for the list of verbs? Shouldn't it then be something like

{
  :idempotent => {
    :verbs => ["GET", "DELETE"], # default: ["GET", "DELETE"]
    :enabled => true # default: false
  }
}

A more generic solution proposed by @voelzmo is to provide a callback in the global settings that decides if a specific call should be retried on basis of request data:

{
  :check_idempotent => lambda {|request| return true if request[:method] == 'GET'}
}

This callback would then be called before issuing every request, defaulting to false if :check_idempotent is not set.

[gildub]

I've tested some initial code with #462.

The default cannot be true because it breaks the tests and might also impact existing deployments. So you have to activate it (:idempotent => yes).

[gildub]

@NautiluX,

Sorry I saw your comment after my latest post.

Well I suppose there are several ways to accomplish the same thing ;).

The important thing, I think, is that :idempotent must be off by default.

[NautiluX]

@gildub, I'm fine with the verb-based proposal as well. :)

[NautiluX]

Hi @gildub, any suggestion when this will get released?

[gildub]

@NautiluX, I'll update the related PR this week.

[gildub]

@NautiluX, I've updated and finished the PR for an initial review. Please feel free to test it for feedback or any issues. Cheers

[NautiluX]

Hi @gildub great to hear, thanks for that! Not sure how quickly we will be able to look at it, but I will forward it to the BOSH OpenStack CPI team.