Arvados

I think Retries and Timeouts is the documentation Terraform referred to. I'm especially looking at the "Client-side rate limiting" section, which says:

If you find that the default behavior does not fit your application's needs, you can disable it with ratelimit.None.

This seems to be how you restore SDK v1-like behavior, which I think is what we want given that we do our own retry handling at the application level.

Aside: That log message does not really mean a-d-c interpreted the error as a capacity error:

                        if err, ok := err.(cloud.QuotaError); ok && err.IsQuotaError() {
                                ...
                        }
                        if err, ok := err.(cloud.CapacityError); ok && err.IsCapacityError() {
                                ...
                        }
                        logger.WithError(err).Error("create failed")

But the main point stands, i.e., the aws sdk's auto-retry behavior is interfering with a-d-c's error handling.

On review, I got a little touchy about the distinction between retries and rate limiting. The specific error we're seeing gets logged when the internal rate limiter has no more tokens which makes me think we should be looking to disable rate limiting, not retrying.

However, note also that happens in a standard retryer method. A look at the corresponding NopRetryer method confirms that, true to its name, it does not talk to a rate limiter at all.

So, if I've followed right, switching to NopRetryer is the change that gets us closest to the SDK v1 behavior that a-d-c expects, and therefore is the best way to resolve this issue. If we're on the same page about this, then this LGTM, thanks.

On review, I got a little touchy about the distinction between retries and rate limiting. The specific error we're seeing gets logged when the internal rate limiter has no more tokens which makes me think we should be looking to disable rate limiting, not retrying.

Sorry, I mentioned auto-retry in the commit message but neglected to call out in my note that auto-retry is also undesirable for a-d-c. It's better for a-d-c to see and report errors immediately, and retry later if the node is still needed. As you noted, switching to NopRetryer gets rid of both.

So, if I've followed right, switching to NopRetryer is the change that gets us closest to the SDK v1 behavior that a-d-c expects, and therefore is the best way to resolve this issue. If we're on the same page about this, then this LGTM, thanks.

Yes.