Skip to content
This repository was archived by the owner on Nov 1, 2023. It is now read-only.

Commit 9f234a1

Browse files
PorgesPorges
authored
Storage performance fixes (#3313)
Noticed a few things while doing #3304, so fixing them in another PR: - Add `CreateNewContainer` which will always create a new container instead of looking for one first. - InstanceId was not being cached properly (the Lazy would be recreated upon every request) — my fault! - Add `GetBlob` beside `GetBlogWithTags` for when we don't need the tags - Make `GetBlogWithTags` do both fetches in parallel
1 parent e68fc45 commit 9f234a1

File tree

6 files changed

+80
-42
lines changed

6 files changed

+80
-42
lines changed

‎src/ApiService/ApiService/Functions/Containers.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ private async Async.Task<HttpResponseData> Post(HttpRequestData req) {
8787

8888
var post = request.OkV;
8989
_logger.LogInformation("creating {ContainerName}", post.Name);
90-
var sas = await _context.Containers.CreateContainer(
90+
var sas = await _context.Containers.GetOrCreateNewContainer(
9191
post.Name,
9292
StorageType.Corpus,
9393
post.Metadata);

‎src/ApiService/ApiService/Functions/Jobs.cs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,16 @@ private async Task<HttpResponseData> Post(HttpRequestData req, FunctionContext c
3636
var userInfo = context.GetUserAuthInfo();
3737

3838
var create = request.OkV;
39-
var cfg = new JobConfig(
40-
Build: create.Build,
41-
Duration: create.Duration,
42-
Logs: create.Logs,
43-
Name: create.Name,
44-
Project: create.Project);
4539

4640
var job = new Job(
4741
JobId: Guid.NewGuid(),
4842
State: JobState.Init,
49-
Config: cfg,
43+
Config: new(
44+
Build: create.Build,
45+
Duration: create.Duration,
46+
Logs: create.Logs,
47+
Name: create.Name,
48+
Project: create.Project),
5049
UserInfo: new(
5150
ObjectId: userInfo.UserInfo.ObjectId,
5251
ApplicationId: userInfo.UserInfo.ApplicationId));
@@ -56,8 +55,11 @@ private async Task<HttpResponseData> Post(HttpRequestData req, FunctionContext c
5655
{ "container_type", "logs" }, // TODO: use ContainerType.Logs enum somehow; needs snake case name
5756
};
5857

59-
var containerName = Container.Parse($"logs-{job.JobId}");
60-
var containerSas = await _context.Containers.CreateContainer(containerName, StorageType.Corpus, metadata);
58+
var containerSas = await _context.Containers.CreateNewContainer(
59+
Container.Parse($"logs-{job.JobId}"),
60+
StorageType.Corpus,
61+
metadata);
62+
6163
if (containerSas is null) {
6264
return await _context.RequestHandling.NotOk(
6365
req,
@@ -76,9 +78,8 @@ private async Task<HttpResponseData> Post(HttpRequestData req, FunctionContext c
7678
return await _context.RequestHandling.NotOk(
7779
req,
7880
Error.Create(
79-
ErrorCode.UNABLE_TO_CREATE,
80-
"unable to create job"
81-
),
81+
ErrorCode.UNABLE_TO_CREATE,
82+
"unable to create job"),
8283
"job");
8384
}
8485

‎src/ApiService/ApiService/onefuzzlib/Containers.cs‎

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System.IO;
22
using System.IO.Compression;
3-
using System.Threading;
43
using System.Threading.Tasks;
54
using ApiService.OneFuzzLib.Orm;
65
using Azure;
@@ -16,11 +15,11 @@ namespace Microsoft.OneFuzz.Service;
1615

1716

1817
public interface IContainers {
19-
public Async.Task<(BinaryData? data, IDictionary<string, string>? tags)> GetBlob(Container container, string name, StorageType storageType);
18+
public Async.Task<BinaryData?> GetBlob(Container container, string name, StorageType storageType);
19+
public Async.Task<(BinaryData? data, IDictionary<string, string>? tags)> GetBlobWithTags(Container container, string name, StorageType storageType);
2020

21-
public Async.Task<Uri?> CreateContainer(Container container, StorageType storageType, IDictionary<string, string>? metadata);
22-
23-
public Async.Task<BlobContainerClient?> GetOrCreateContainerClient(Container container, StorageType storageType, IDictionary<string, string>? metadata);
21+
public Async.Task<Uri?> CreateNewContainer(Container container, StorageType storageType, IDictionary<string, string>? metadata);
22+
public Async.Task<Uri?> GetOrCreateNewContainer(Container container, StorageType storageType, IDictionary<string, string>? metadata);
2423

2524
public Async.Task<BlobContainerClient?> FindContainer(Container container, StorageType storageType);
2625
public Async.Task<bool> DeleteContainerIfExists(Container container, StorageType storageType);
@@ -65,14 +64,6 @@ public Containers(
6564
_config = config;
6665
_cache = cache;
6766

68-
_getInstanceId = new Lazy<Async.Task<Guid>>(async () => {
69-
var (data, tags) = await GetBlob(WellKnownContainers.BaseConfig, "instance_id", StorageType.Config);
70-
if (data == null) {
71-
throw new Exception("Blob Not Found");
72-
}
73-
74-
return Guid.Parse(data.ToString());
75-
}, LazyThreadSafetyMode.PublicationOnly);
7667
}
7768

7869
public async Async.Task<Uri?> GetFileUrl(Container container, string name, StorageType storageType) {
@@ -83,22 +74,45 @@ public Containers(
8374
return client.GetBlobClient(name).Uri;
8475
}
8576

86-
public async Async.Task<(BinaryData? data, IDictionary<string, string>? tags)> GetBlob(Container container, string name, StorageType storageType) {
77+
public async Async.Task<(BinaryData? data, IDictionary<string, string>? tags)> GetBlobWithTags(Container container, string name, StorageType storageType) {
8778
var client = await FindContainer(container, storageType);
8879
if (client == null) {
8980
return (null, null);
9081
}
9182

83+
var blobClient = client.GetBlobClient(name);
9284
try {
93-
var blobClient = client.GetBlobClient(name);
94-
var tags = await blobClient.GetTagsAsync();
95-
return ((await blobClient.DownloadContentAsync()).Value.Content, tags.Value.Tags);
85+
var (tags, content) = await (blobClient.GetTagsAsync(), blobClient.DownloadContentAsync());
86+
return (content.Value.Content, tags.Value.Tags);
9687
} catch (RequestFailedException) {
9788
return (null, null);
9889
}
9990
}
10091

101-
public async Task<Uri?> CreateContainer(Container container, StorageType storageType, IDictionary<string, string>? metadata) {
92+
public async Async.Task<BinaryData?> GetBlob(Container container, string name, StorageType storageType) {
93+
var client = await FindContainer(container, storageType);
94+
if (client == null) {
95+
return null;
96+
}
97+
98+
var blobClient = client.GetBlobClient(name);
99+
try {
100+
return (await blobClient.DownloadContentAsync()).Value.Content;
101+
} catch (RequestFailedException) {
102+
return null;
103+
}
104+
}
105+
106+
public async Task<Uri?> CreateNewContainer(Container container, StorageType storageType, IDictionary<string, string>? metadata) {
107+
var client = await CreateNewContainerClient(container, storageType, metadata);
108+
if (client is null) {
109+
return null;
110+
}
111+
112+
return GetContainerSasUrlService(client, _containerCreatePermissions);
113+
}
114+
115+
public async Task<Uri?> GetOrCreateNewContainer(Container container, StorageType storageType, IDictionary<string, string>? metadata) {
102116
var client = await GetOrCreateContainerClient(container, storageType, metadata);
103117
if (client is null) {
104118
return null;
@@ -113,12 +127,24 @@ private static readonly BlobContainerSasPermissions _containerCreatePermissions
113127
| BlobContainerSasPermissions.Delete
114128
| BlobContainerSasPermissions.List;
115129

116-
public async Task<BlobContainerClient?> GetOrCreateContainerClient(Container container, StorageType storageType, IDictionary<string, string>? metadata) {
130+
public async Task<BlobContainerClient?> GetOrCreateContainerClient(
131+
Container container,
132+
StorageType storageType,
133+
IDictionary<string, string>? metadata) {
134+
117135
var containerClient = await FindContainer(container, StorageType.Corpus);
118136
if (containerClient is not null) {
119137
return containerClient;
120138
}
121139

140+
return await CreateNewContainerClient(container, storageType, metadata);
141+
}
142+
143+
public async Task<BlobContainerClient?> CreateNewContainerClient(
144+
Container container,
145+
StorageType storageType,
146+
IDictionary<string, string>? metadata) {
147+
122148
var account = _storage.ChooseAccount(storageType);
123149
var client = await _storage.GetBlobServiceClientForAccount(account);
124150
var containerName = _config.OneFuzzStoragePrefix + container;
@@ -290,8 +316,19 @@ private async Async.Task SaveBlobInternal(Container container, string name, stri
290316
}
291317
}
292318

293-
public virtual Async.Task<Guid> GetInstanceId() => _getInstanceId.Value;
294-
private readonly Lazy<Async.Task<Guid>> _getInstanceId;
319+
private static readonly object _instanceIdKey = new();
320+
public virtual Async.Task<Guid> GetInstanceId() {
321+
return _cache.GetOrCreateAsync(_instanceIdKey, async ce => {
322+
ce.AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(7); // should never change
323+
324+
var data = await GetBlob(WellKnownContainers.BaseConfig, "instance_id", StorageType.Config);
325+
if (data == null) {
326+
throw new Exception("Couldn't find instance_id blob");
327+
}
328+
329+
return Guid.Parse(data.ToString());
330+
});
331+
}
295332

296333
public Uri GetContainerSasUrlService(
297334
BlobContainerClient client,

‎src/ApiService/ApiService/onefuzzlib/Events.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public virtual void LogEvent(BaseEvent anEvent) {
9393
}
9494

9595
public async Async.Task<OneFuzzResult<DownloadableEventMessage>> GetDownloadableEvent(Guid eventId) {
96-
var (data, tags) = await _containers.GetBlob(WellKnownContainers.Events, eventId.ToString(), StorageType.Corpus);
96+
var (data, tags) = await _containers.GetBlobWithTags(WellKnownContainers.Events, eventId.ToString(), StorageType.Corpus);
9797
if (data == null) {
9898
return OneFuzzResult<DownloadableEventMessage>.Error(ErrorCode.UNABLE_TO_FIND, $"Could not find container for event with id {eventId}");
9999
}

‎src/ApiService/IntegrationTests/ContainersTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ public async Async.Task DeleteExpiredBlobsDoesNotTouchUntaggedBlobs() {
247247

248248
TestFeatureManagerSnapshot.AddFeatureFlag(FeatureFlagConstants.EnableDryRunBlobRetention, enabled: false);
249249

250-
_ = await Context.Containers.CreateContainer(testContainer, StorageType.Corpus, null);
250+
_ = await Context.Containers.CreateNewContainer(testContainer, StorageType.Corpus, null);
251251
await Context.Containers.SaveBlob(testContainer, expirableBlobName, string.Empty, StorageType.Corpus, DateOnly.MinValue);
252252
await Context.Containers.SaveBlob(testContainer, nonExpirableBlobName, string.Empty, StorageType.Corpus);
253253

‎src/ApiService/IntegrationTests/JinjaToScribanMigrationTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ protected JinjaToScribanMigrationTestBase(ITestOutputHelper output, IStorage sto
2626
[Fact]
2727
public async Async.Task Dry_Run_Does_Not_Make_Changes() {
2828
var notificationContainer = Container.Parse("abc123");
29-
var _ = await Context.Containers.CreateContainer(notificationContainer, StorageType.Corpus, null);
29+
var _ = await Context.Containers.CreateNewContainer(notificationContainer, StorageType.Corpus, null);
3030
var r = await Context.NotificationOperations.Create(
3131
notificationContainer,
3232
MigratableAdoTemplate(),
@@ -61,7 +61,7 @@ public async Async.Task Dry_Run_Does_Not_Make_Changes() {
6161
[Fact]
6262
public async Async.Task Migration_Happens_When_Not_Dry_run() {
6363
var notificationContainer = Container.Parse("abc123");
64-
var _ = await Context.Containers.CreateContainer(notificationContainer, StorageType.Corpus, null);
64+
var _ = await Context.Containers.CreateNewContainer(notificationContainer, StorageType.Corpus, null);
6565
var r = await Context.NotificationOperations.Create(
6666
notificationContainer,
6767
MigratableAdoTemplate(),
@@ -140,7 +140,7 @@ public async Async.Task All_ADO_Fields_Are_Migrated() {
140140
"{% if org %} comment {% endif %}"
141141
);
142142

143-
var _ = await Context.Containers.CreateContainer(notificationContainer, StorageType.Corpus, null);
143+
var _ = await Context.Containers.CreateNewContainer(notificationContainer, StorageType.Corpus, null);
144144
var r = await Context.NotificationOperations.Create(
145145
notificationContainer,
146146
adoTemplate,
@@ -189,7 +189,7 @@ public async Async.Task All_Github_Fields_Are_Migrated() {
189189
var githubTemplate = MigratableGithubTemplate();
190190

191191
var notificationContainer = Container.Parse("abc123");
192-
var _ = await Context.Containers.CreateContainer(notificationContainer, StorageType.Corpus, null);
192+
var _ = await Context.Containers.CreateNewContainer(notificationContainer, StorageType.Corpus, null);
193193
var r = await Context.NotificationOperations.Create(
194194
notificationContainer,
195195
githubTemplate,
@@ -241,7 +241,7 @@ public async Async.Task All_Github_Fields_Are_Migrated() {
241241
public async Async.Task Teams_Template_Not_Migrated() {
242242
var teamsTemplate = GetTeamsTemplate();
243243
var notificationContainer = Container.Parse("abc123");
244-
var _ = await Context.Containers.CreateContainer(notificationContainer, StorageType.Corpus, null);
244+
var _ = await Context.Containers.CreateNewContainer(notificationContainer, StorageType.Corpus, null);
245245
var r = await Context.NotificationOperations.Create(
246246
notificationContainer,
247247
teamsTemplate,
@@ -273,7 +273,7 @@ public async Async.Task Teams_Template_Not_Migrated() {
273273
[Fact]
274274
public async Async.Task Can_Migrate_Multiple_Notification_Configs() {
275275
var notificationContainer = Container.Parse("abc123");
276-
var _ = await Context.Containers.CreateContainer(notificationContainer, StorageType.Corpus, null);
276+
var _ = await Context.Containers.CreateNewContainer(notificationContainer, StorageType.Corpus, null);
277277

278278
var teamsTemplate = GetTeamsTemplate();
279279
var r = await Context.NotificationOperations.Create(

0 commit comments

Comments
 (0)