-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SqlReader Retry Logic #233
Comments
Are you sure that is desirable? Retries can take down a server, and lengthen transient failures. For example, imagine a server temporarily stops responding. Without retries, a few images fail to appear, but no additional load is placed on the server except end-users hitting refresh. With retries, a backlog of retries is added to the request queue, which makes it more likely that the server will continue failing requests rather than make progress on shortening the queue and reducing the response time. With subscription quotas, this problem gets even worse. There's not that much fetch code: https://github.com/imazen/resizer/blob/develop/Plugins/SqlReader/SqlReader.cs#L145-L218 It seems like it would be trivial to implement. But I think I would suggest such a feature be off by default. |
Yes, retry of transient errors is desirable and recommended for Azure SQL Database. As this request specifically targets documented[1] transient errors, which typically resolve quickly, impact upon services need not be severe. Impact can be lessened further by exponential backoff, and even further by a cooperative retry policy — stop retrying when a heuristic detects that it isn't helping. Off-by-default is a safe choice. Some configurability of the retry policy would be nice as well:
[1] See code links in original post, and also this MS article. |
Okay. Keep in mind browsers have their own request timeouts. I'd be open to a pull request for this if the logic is clear and has tests. |
I might be able to put together a PR, though not quickly. |
When using a PaaS database product like Azure SQL Database, occasional transient errors occur more frequently than with a traditional on-premises server. The usual solution is to add retry logic.
Good sources on what errors are retryable are in here and here. If you would like a ready-made
SqlConnection
wrapper that implements retries, I can probably arrange to get you the one my client uses — I wrote it. DM me on twitter @sharpjs to discuss further.Here is an example of one such error that came through my client's monitoring today:
The text was updated successfully, but these errors were encountered: