-
Notifications
You must be signed in to change notification settings - Fork 583
Automatic retries #2183
Description
I'd like to request that Gophercloud support (an optional) automatic retry mechanism. I've seen this mentioned and rejected a couple times, but I'd like to make additional arguments for why it would be good if this was supported.
There are status codes and errors that we always want to retry - timeouts, internal service error (500) from unreliable upstream providers, maybe reauthentication when creds expire, other random network glitches, etc. In order for us to retry these on every call to every openstack service we use, even assuming we only call each once, it's still 50+ method calls that need to be wrapped. We could use go's reflection to make a wrapper method but at the same time lose type safety and we'd still need to go in and touch each call.
Gophercloud's code is in a much better position to do this: all requests go through the doRequest method in the client object which deals with generic requests (and retrying already exists for one error code). This isn't something that can be trivially worked around downstream - ProviderClient is a concrete type so a custom implementation can't be provided and there's no way to inject custom request processing code (AFAICT) since each of the request methods call client.Request directly.
FWIW other iaas api libraries (AWS, GCP, Azure) etc all do 3-5 retries by default to my knowledge.
I assume there were likely concerns about duplicate resources being created or requests failing because they caused a state change before being retried, etc. For the most part we're creating resources, listing resources, and deleting resources (the latter of which we mostly log and ignore the result) where this doesn't matter or else would be an issue regardless of retries. For other apis (attaching volumes, maybe) which involve state changes, handling an error from a duplicate execution wouldn't be harder than manually retrying in the case of a temporary error. Regardless though I think making it optional would be reasonable.