[Storage] updating to 11.1.4; handling storage network timeouts #2486
Conversation
| } | ||
| catch (StorageException ex) when (ex.InnerException is OperationCanceledException && !cancellationToken.IsCancellationRequested) // network timeout | ||
| { | ||
| Logger.StorageTimeout(logger, operationName, clientRequestId, sw.ElapsedMilliseconds); |
There was a problem hiding this comment.
Most of the other changes were to allow me to plumb a logger through to here so all of this was centralized.
|
|
||
| public static async Task<T> ExecuteWithTimeout<T>(string operationName, string clientRequestId, IWebJobsExceptionHandler exceptionHandler, Func<Task<T>> operation) | ||
| public static async Task<T> ExecuteWithTimeout<T>(string operationName, string clientRequestId, | ||
| IWebJobsExceptionHandler exceptionHandler, ILogger logger, CancellationToken cancellationToken, Func<Task<T>> operation) |
There was a problem hiding this comment.
This is kind of clunky, but this cancellationToken isn't used to cancel anything. But in order to determine if it was a network timeout (for logging only), I need to know if someone canceled the operation directly. Since this is hopefully a short-lived API, I'm okay with it being odd for now and this whole class will go away in a future update.
| @@ -147,9 +147,9 @@ private async Task RunAsync(CancellationToken cancellationToken) | |||
| TaskSeriesCommandResult result = await _command.ExecuteAsync(cancellationToken); | |||
| wait = result.Wait; | |||
| } | |||
| catch (Exception ex) when (ex.InnerException is TaskCanceledException) | |||
| catch (Exception ex) when (ex.InnerException is OperationCanceledException) | |||
There was a problem hiding this comment.
Took me a while to realize that moving this to OperationCanceledException did everything I needed. The new behavior throws an OperationCanceledException wrapped in a StorageException if the network hangs for 100 seconds. Now, that exception will bubble up to here and the loop will start over, just as we'd want it to. (and it behaves the same as before as TaskCanceledException : OperationCanceledException).
|
Thanks! Yeah those host changes are fine. That TaskSeriesTimer is source-shared across a couple projects so they'll all get that update internally (which is how I can ship this in the extension without needing a host deployment). |
Storage 11.1.4 now cancels network timeouts after 100 seconds by default: Azure/azure-storage-net#985
Our TimeoutHandler workaround is likely not needed at all anymore, however, I wanted to leave it in just to make sure. So I changed this to:
If things go well and we never see the TimeoutHandler fire anymore, we can remove it completely and simplify these calls in a future release.