fog-openstack icon indicating copy to clipboard operation
fog-openstack copied to clipboard

Auto-retry idempotent requests

Open NautiluX opened this issue 6 years ago • 13 comments

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.

NautiluX avatar Oct 15 '18 09:10 NautiluX

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}.

gildub avatar Oct 23 '18 00:10 gildub

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.

NautiluX avatar Oct 30 '18 07:10 NautiluX

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?

gildub avatar Oct 31 '18 06:10 gildub

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.

NautiluX avatar Oct 31 '18 12:10 NautiluX

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?

gildub avatar Nov 01 '18 10:11 gildub

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.

NautiluX avatar Nov 02 '18 09:11 NautiluX

I've tested some initial code with https://github.com/fog/fog-openstack/pull/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 avatar Nov 02 '18 12:11 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.

gildub avatar Nov 07 '18 05:11 gildub

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

NautiluX avatar Nov 07 '18 08:11 NautiluX

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

NautiluX avatar Mar 12 '19 07:03 NautiluX

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

gildub avatar Mar 18 '19 15:03 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

gildub avatar Mar 20 '19 09:03 gildub

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.

NautiluX avatar Apr 15 '19 06:04 NautiluX