Helix JobSender's UploadFileAsync function has no error handling / retry for Helix payload upload

[MattGal]

While it fails infrequently, this line : https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Helix/JobSender/StorageHelpers/ContainerBase.cs#L22 has no retry for cases of network connectivity issues between the build machine and Azure storage, as seen in dotnet/corefx#41933 (comment) .

Since the executing code here already has write permission for a specific blob, I think a universal retry (possibly with flags to allow overwrite) makes the most sense. As it's only happened once that I've seen thus far, not marking as FR (but if whoever triages this thinks so, feel free)

[markwilkie]

Is this also another instance where using a Azure SDK would have avoided this?

[MattGal]

Is this also another instance where using a Azure SDK would have avoided this?

Not in this case, though it's perhaps a nice argument for them providing an API with built-in retry (I didn't check, it's possible that has been added). From the call stack.

##[error].packages\microsoft.dotnet.helix.sdk\5.0.0-beta.19518.2\tools\Microsoft.DotNet.Helix.Sdk.MonoQueue.targets(47,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) StorageException: An error occurred while sending the request.
   at Microsoft.WindowsAzure.Storage.Core.Executor.Executor.ExecuteAsyncInternal[T](RESTCommand`1 cmd, IRetryPolicy policy, OperationContext operationContext, CancellationToken token)
   at Microsoft.WindowsAzure.Storage.Blob.CloudBlockBlob.<>c__DisplayClass28_0.<<UploadFromStreamAsyncHelper>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---

... one can see it's already using the public Azure storage SDK.

[MattGal]

(Reading https://github.com/Azure/azure-storage-net/blob/master/Lib/ClassLibraryCommon/Blob/CloudBlockBlob.cs it would seem all variations of UploadFromStreamAsync() have no retry logic built in)

[markwilkie]

If this is Peter's team, I wonder if we can request them to add it?

[MattGal]

Yes, or provide a code snippet for best practices; it's clear that even if every machine used is in an Azure data center this kind of thing can and does happen.

[alexperovich]

This is using the old azure storage sdk that has no retry. The new storage sdks from Peter's team have retry built in.

[markwilkie]

As I recall, we've moved to the latest storage sdk.

[MattGal]

As I recall, we've moved to the latest storage sdk.

Yup, did this some months ago now and haven't seen (this particular) issue repro as well since.