Skip to content

Commit 65c10dd

Browse files
balag0brettsam
authored andcommitted
Retry storage failures when downloading zip used in Run-From-Package
1 parent 690321b commit 65c10dd

File tree

7 files changed

+129
-31
lines changed

7 files changed

+129
-31
lines changed

src/WebJobs.Script.WebHost/ContainerManagement/LinuxContainerInitializationHostService.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Microsoft.Extensions.Hosting;
1010
using Microsoft.Extensions.Logging;
1111
using Microsoft.WindowsAzure.Storage.Blob;
12+
using Microsoft.WindowsAzure.Storage.RetryPolicies;
1213
using Newtonsoft.Json;
1314

1415
namespace Microsoft.Azure.WebJobs.Script.WebHost.ContainerManagement
@@ -104,9 +105,14 @@ public virtual async Task<string> Read(string uri)
104105
// When the blob doesn't exist it just means the container is waiting for specialization.
105106
// Don't treat this as a failure.
106107
var cloudBlockBlob = new CloudBlockBlob(new Uri(uri));
107-
if (await cloudBlockBlob.ExistsAsync(null, null, _cancellationToken))
108+
109+
var blobRequestOptions = new BlobRequestOptions
110+
{
111+
RetryPolicy = new LinearRetry(TimeSpan.FromMilliseconds(500), 3)
112+
};
113+
if (await cloudBlockBlob.ExistsAsync(blobRequestOptions, null, _cancellationToken))
108114
{
109-
return await cloudBlockBlob.DownloadTextAsync(null, null, null, null, _cancellationToken);
115+
return await cloudBlockBlob.DownloadTextAsync(null, null, blobRequestOptions, null, _cancellationToken);
110116
}
111117

112118
return string.Empty;

src/WebJobs.Script.WebHost/Management/InstanceManager.cs

Lines changed: 73 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.IO;
78
using System.IO.Compression;
89
using System.Net.Http;
910
using System.Threading.Tasks;
11+
using Microsoft.Azure.WebJobs.Script.Diagnostics;
1012
using Microsoft.Azure.WebJobs.Script.WebHost.Configuration;
1113
using Microsoft.Azure.WebJobs.Script.WebHost.Models;
1214
using Microsoft.Extensions.Logging;
@@ -20,17 +22,19 @@ public class InstanceManager : IInstanceManager
2022
private static HostAssignmentContext _assignmentContext;
2123

2224
private readonly ILogger _logger;
25+
private readonly IMetricsLogger _metricsLogger;
2326
private readonly IEnvironment _environment;
2427
private readonly IOptionsFactory<ScriptApplicationHostOptions> _optionsFactory;
2528
private readonly HttpClient _client;
2629
private readonly IScriptWebHostEnvironment _webHostEnvironment;
2730

2831
public InstanceManager(IOptionsFactory<ScriptApplicationHostOptions> optionsFactory, HttpClient client, IScriptWebHostEnvironment webHostEnvironment,
29-
IEnvironment environment, ILogger<InstanceManager> logger)
32+
IEnvironment environment, ILogger<InstanceManager> logger, IMetricsLogger metricsLogger)
3033
{
3134
_client = client ?? throw new ArgumentNullException(nameof(client));
3235
_webHostEnvironment = webHostEnvironment ?? throw new ArgumentNullException(nameof(webHostEnvironment));
3336
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
37+
_metricsLogger = metricsLogger;
3438
_environment = environment ?? throw new ArgumentNullException(nameof(environment));
3539
_optionsFactory = optionsFactory ?? throw new ArgumentNullException(nameof(optionsFactory));
3640
}
@@ -80,21 +84,40 @@ public async Task<string> ValidateContext(HostAssignmentContext assignmentContex
8084
{
8185
_logger.LogInformation($"Validating host assignment context (SiteId: {assignmentContext.SiteId}, SiteName: '{assignmentContext.SiteName}')");
8286

83-
var zipUrl = assignmentContext.ZipUrl;
84-
if (!string.IsNullOrEmpty(zipUrl))
87+
string error = null;
88+
HttpResponseMessage response = null;
89+
try
8590
{
86-
// make sure the zip uri is valid and accessible
87-
var request = new HttpRequestMessage(HttpMethod.Head, zipUrl);
88-
var response = await _client.SendAsync(request);
89-
if (!response.IsSuccessStatusCode)
91+
var zipUrl = assignmentContext.ZipUrl;
92+
if (!string.IsNullOrEmpty(zipUrl))
9093
{
91-
string error = $"Invalid zip url specified (StatusCode: {response.StatusCode})";
92-
_logger.LogError(error);
93-
return error;
94+
// make sure the zip uri is valid and accessible
95+
await Utility.InvokeWithRetriesAsync(async () =>
96+
{
97+
try
98+
{
99+
using (_metricsLogger.LatencyEvent(MetricEventNames.LinuxContainerSpecializationZipHead))
100+
{
101+
var request = new HttpRequestMessage(HttpMethod.Head, zipUrl);
102+
response = await _client.SendAsync(request);
103+
response.EnsureSuccessStatusCode();
104+
}
105+
}
106+
catch (Exception e)
107+
{
108+
_logger.LogError(e, $"{MetricEventNames.LinuxContainerSpecializationZipHead} failed");
109+
throw;
110+
}
111+
}, maxRetries: 2, retryInterval: TimeSpan.FromSeconds(0.3)); // Keep this less than ~1s total
94112
}
95113
}
114+
catch (Exception e)
115+
{
116+
error = $"Invalid zip url specified (StatusCode: {response?.StatusCode})";
117+
_logger.LogError(e, "ValidateContext failed");
118+
}
96119

97-
return null;
120+
return error;
98121
}
99122

100123
private async Task Assign(HostAssignmentContext assignmentContext)
@@ -140,32 +163,55 @@ private async Task ApplyContext(HostAssignmentContext assignmentContext)
140163
var filePath = Path.GetTempFileName();
141164
await DownloadAsync(zipUri, filePath);
142165

143-
_logger.LogInformation($"Extracting files to '{options.ScriptPath}'");
144-
ZipFile.ExtractToDirectory(filePath, options.ScriptPath, overwriteFiles: true);
145-
_logger.LogInformation($"Zip extraction complete");
166+
using (_metricsLogger.LatencyEvent(MetricEventNames.LinuxContainerSpecializationZipExtract))
167+
{
168+
_logger.LogInformation($"Extracting files to '{options.ScriptPath}'");
169+
ZipFile.ExtractToDirectory(filePath, options.ScriptPath, overwriteFiles: true);
170+
_logger.LogInformation($"Zip extraction complete");
171+
}
146172
}
147173
}
148174

149175
private async Task DownloadAsync(Uri zipUri, string filePath)
150176
{
151-
var zipPath = $"{zipUri.Authority}{zipUri.AbsolutePath}";
152-
_logger.LogInformation($"Downloading zip contents from '{zipPath}' to temp file '{filePath}'");
177+
string cleanedUrl;
178+
Utility.TryCleanUrl(zipUri.AbsoluteUri, out cleanedUrl);
179+
180+
_logger.LogInformation($"Downloading zip contents from '{cleanedUrl}' to temp file '{filePath}'");
181+
182+
HttpResponseMessage response = null;
153183

154-
var response = await _client.GetAsync(zipUri);
155-
if (!response.IsSuccessStatusCode)
184+
await Utility.InvokeWithRetriesAsync(async () =>
156185
{
157-
string error = $"Error downloading zip content {zipPath}";
158-
_logger.LogError(error);
159-
throw new InvalidDataException(error);
160-
}
186+
try
187+
{
188+
using (_metricsLogger.LatencyEvent(MetricEventNames.LinuxContainerSpecializationZipDownload))
189+
{
190+
var request = new HttpRequestMessage(HttpMethod.Get, zipUri);
191+
response = await _client.SendAsync(request);
192+
response.EnsureSuccessStatusCode();
193+
}
194+
}
195+
catch (Exception e)
196+
{
197+
string error = $"Error downloading zip content {cleanedUrl}";
198+
_logger.LogError(e, error);
199+
throw;
200+
}
161201

162-
using (var content = await response.Content.ReadAsStreamAsync())
163-
using (var stream = new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None, bufferSize: 4096, useAsync: true))
202+
_logger.LogInformation($"{response.Content.Headers.ContentLength} bytes downloaded");
203+
}, 2, TimeSpan.FromSeconds(0.5));
204+
205+
using (_metricsLogger.LatencyEvent(MetricEventNames.LinuxContainerSpecializationZipWrite))
164206
{
165-
await content.CopyToAsync(stream);
166-
}
207+
using (var content = await response.Content.ReadAsStreamAsync())
208+
using (var stream = new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None, bufferSize: 4096, useAsync: true))
209+
{
210+
await content.CopyToAsync(stream);
211+
}
167212

168-
_logger.LogInformation($"{response.Content.Headers.ContentLength} bytes downloaded");
213+
_logger.LogInformation($"{response.Content.Headers.ContentLength} bytes written");
214+
}
169215
}
170216

171217
public IDictionary<string, string> GetInstanceInfo()

src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public static void AddWebJobsScriptHost(this IServiceCollection services, IConfi
7070
services.AddSingleton<IScriptWebHostEnvironment, ScriptWebHostEnvironment>();
7171
services.AddSingleton<IStandbyManager, StandbyManager>();
7272
services.TryAddSingleton<IScriptHostBuilder, DefaultScriptHostBuilder>();
73+
services.AddSingleton<IMetricsLogger, WebHostMetricsLogger>();
7374

7475
// Linux container services
7576
services.AddLinuxContainerServices();

src/WebJobs.Script/Diagnostics/MetricEventNames.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,11 @@ public static class MetricEventNames
3737
public const string SecretManagerAddOrUpdateFunctionSecret = "secretmanager.addorupdatefunctionsecret.{0}";
3838
public const string SecretManagerSetMasterKey = "secretmanager.setmasterkey.{0}";
3939
public const string SecretManagerPurgeOldSecrets = "secretmanager.purgeoldsecrets.{0}";
40+
41+
// Linux container specialization events
42+
public const string LinuxContainerSpecializationZipExtract = "linux.container.specialization.zip.extract";
43+
public const string LinuxContainerSpecializationZipDownload = "linux.container.specialization.zip.download";
44+
public const string LinuxContainerSpecializationZipWrite = "linux.container.specialization.zip.write";
45+
public const string LinuxContainerSpecializationZipHead = "linux.container.specialization.zip.head";
4046
}
4147
}

src/WebJobs.Script/Utility.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,24 @@ public static bool CheckAppOffline(string scriptPath)
521521
return false;
522522
}
523523

524+
public static bool TryCleanUrl(string url, out string cleaned)
525+
{
526+
cleaned = null;
527+
528+
Uri uri = null;
529+
if (Uri.TryCreate(url, UriKind.Absolute, out uri))
530+
{
531+
cleaned = $"{uri.Scheme}://{uri.Host}{uri.AbsolutePath}";
532+
if (uri.Query.Length > 0)
533+
{
534+
cleaned += "...";
535+
}
536+
return true;
537+
}
538+
539+
return false;
540+
}
541+
524542
private class FilteredExpandoObjectConverter : ExpandoObjectConverter
525543
{
526544
public override bool CanWrite => true;

test/WebJobs.Script.Tests.Integration/Management/InstanceManagerTests.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public InstanceManagerTests()
3838
_scriptWebEnvironment = new ScriptWebHostEnvironment(_environment);
3939

4040
var optionsFactory = new TestOptionsFactory<ScriptApplicationHostOptions>(new ScriptApplicationHostOptions());
41-
_instanceManager = new InstanceManager(optionsFactory, _httpClient, _scriptWebEnvironment, _environment, loggerFactory.CreateLogger<InstanceManager>());
41+
_instanceManager = new InstanceManager(optionsFactory, _httpClient, _scriptWebEnvironment, _environment, loggerFactory.CreateLogger<InstanceManager>(), new TestMetricsLogger());
4242

4343
InstanceManager.Reset();
4444
}
@@ -162,7 +162,10 @@ public async Task ValidateContext_InvalidZipUrl_ReturnsError()
162162
var logs = _loggerProvider.GetAllLogMessages().Select(p => p.FormattedMessage).ToArray();
163163
Assert.Collection(logs,
164164
p => Assert.StartsWith("Validating host assignment context (SiteId: 1234, SiteName: 'TestSite')", p),
165-
p => Assert.StartsWith("Invalid zip url specified (StatusCode: NotFound)", p));
165+
p => Assert.StartsWith("linux.container.specialization.zip.head failed", p),
166+
p => Assert.StartsWith("linux.container.specialization.zip.head failed", p),
167+
p => Assert.StartsWith("linux.container.specialization.zip.head failed", p),
168+
p => Assert.StartsWith("ValidateContext failed", p));
166169
}
167170

168171
[Fact]

test/WebJobs.Script.Tests/UtilityTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,5 +322,23 @@ public void IsNullable_ReturnsExpectedResult(Type type, bool expected)
322322
{
323323
Assert.Equal(expected, Utility.IsNullable(type));
324324
}
325+
326+
[Theory]
327+
[InlineData("", null, false)]
328+
[InlineData(null, null, false)]
329+
[InlineData("http://storage.blob.core.windows.net/functions/func.zip?sr=c&si=policy&sig=f%2BGLvBih%2BoFuQvckBSHWKMXwqGJHlPkESmZh9pjnHuc%3D",
330+
"http://storage.blob.core.windows.net/functions/func.zip...", true)]
331+
[InlineData("http://storage.blob.core.windows.net/functions/func.zip",
332+
"http://storage.blob.core.windows.net/functions/func.zip", true)]
333+
[InlineData("https://storage.blob.core.windows.net/functions/func.zip",
334+
"https://storage.blob.core.windows.net/functions/func.zip", true)]
335+
[InlineData("https://storage.blob.core.windows.net/functions/func.zip?",
336+
"https://storage.blob.core.windows.net/functions/func.zip...", true)]
337+
public void CleanUrlTests(string url, string expectedCleanUrl, bool cleanResult)
338+
{
339+
string cleanedUrl;
340+
Assert.Equal(cleanResult, Utility.TryCleanUrl(url, out cleanedUrl));
341+
Assert.Equal(expectedCleanUrl, cleanedUrl);
342+
}
325343
}
326344
}

0 commit comments

Comments
 (0)