Merged PR 742087: Remove version restriction from blob options and centralize HttpClient

Centralizes the storage options further to make sure that all across cache we're not going over the connection limit. Also removes the version limit we're placing on the storage sdk because of our old version of azurite.

This also fixes a bug when using managed identities that would cause the user to not utilize the singleton HttpClient, potentially running into the FD limit

Author: jbayardo

Public/Src/Cache/ContentStore/Distributed/Blob/ShardedBlobCacheTopology.cs

@@ -43,15 +43,6 @@ public record Configuration(
     private readonly BlobCacheContainerName[] _containers;
     private readonly IShardingScheme<int, BlobCacheStorageAccountName> _scheme;
 
-    /// <remarks>
-    /// We will reuse an HttpClient for the transport backing the blob clients. HttpClient is meant to be reused anyway
-    /// (https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-7.0#instancing)
-    /// but crucially we have the need to configure the amount of open connections: when using the defaults,
-    /// the number of connections is unbounded, and we have observed builds where there end up being tens of thousands
-    /// of open sockets, which can (and did) hit the per-process limit of open files, crashing the engine.
-    /// </summary>
-    private readonly HttpClient _httpClient;
-
     private readonly record struct Location(BlobCacheStorageAccountName Account, BlobCacheContainerName Container);
 
     /// <summary>
@@ -75,19 +66,6 @@ public ShardedBlobCacheTopology(Configuration configuration)
 
         _scheme = _configuration.ShardingScheme.Create();
         _containers = GenerateContainerNames(_configuration.Universe, _configuration.Namespace, _configuration.ShardingScheme);
-
-        _httpClient = new HttpClient(
-            new HttpClientHandler()
-            {
-                // If left unbounded, we have observed spikes of >65k open sockets (at which point we hit
-                // the OS limit of open files for the process - on Linux, where sockets count as files).
-                // Running builds where we limit this value all the way down to 100 didn't see
-                // any noticeable performance impact, so 30k shouldn't pose a problem.
-                // The configurable limit is per-client and per-server, but because we will reuse this HttpClient
-                // for all BlobClients and the 'server' (blob storage endpoint) is also always the same,
-                // we are effectively limiting the number of open connections in general.
-                MaxConnectionsPerServer = 30_000
-            });
     }
 
     internal static BlobCacheContainerName[] GenerateContainerNames(string universe, string @namespace, ShardingScheme scheme)
@@ -207,13 +185,7 @@ private Task<Result<BlobContainerClient>> CreateClientAsync(OperationContext con
             async context =>
             {
                 var credentials = await _configuration.SecretsProvider.RetrieveBlobCredentialsAsync(context, account);
-
-                BlobClientOptions blobClientOptions = new(BlobClientOptions.ServiceVersion.V2021_02_12)
-                {
-                    Transport = new HttpClientTransport(_httpClient)
-                };
-
-                var containerClient = credentials.CreateContainerClient(container.ContainerName, blobClientOptions);
+                var containerClient = credentials.CreateContainerClient(container.ContainerName);
 
                 try
                 {

Public/Src/Cache/ContentStore/DistributedTest/Redis/AzuriteStorageProcess.cs

@@ -264,7 +264,7 @@ private void Start(List<string> accounts = null)
                 _fileSystem.CreateDirectory(storageServerWorkspacePath);
                 _portNumber = PortExtensions.GetNextAvailablePort();
 
-                var args = $"--blobPort {_portNumber} --location {storageServerWorkspacePath} --skipApiVersionCheck";
+                var args = $"--blobPort {_portNumber} --location {storageServerWorkspacePath} --skipApiVersionCheck --silent --loose";
                 _logger.Debug($"Running cmd=[{storageServerPath} {args}]");
 
                 _process = new ProcessUtility(storageServerPath, args, createNoWindow: true, workingDirectory: Path.GetDirectoryName(storageServerPath), environment: CreateEnvironment(accounts));

Public/Src/Cache/ContentStore/Interfaces/Auth/BlobClientOptionsFactory.cs

@@ -0,0 +1,45 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System.Net.Http;
+using Azure.Core.Pipeline;
+using Azure.Storage.Blobs;
+
+namespace BuildXL.Cache.ContentStore.Interfaces.Auth;
+
+/// <summary>
+/// Centralized point for common methods pertaining to Azure Storage authentication.
+/// </summary>
+internal static class BlobClientOptionsFactory
+{
+    /// <summary>
+    /// We will reuse an HttpClient for the transport backing the blob clients. HttpClient is meant to be reused anyway
+    /// (https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-7.0#instancing)
+    /// but crucially we have the need to configure the amount of open connections: when using the defaults,
+    /// the number of connections is unbounded, and we have observed builds where there end up being tens of thousands
+    /// of open sockets, which can (and did) hit the per-process limit of open files, crashing the engine.
+    /// </summary>
+    private static readonly HttpClient HttpClient = new(
+        new HttpClientHandler
+        {
+            // If left unbounded, we have observed spikes of >65k open sockets (at which point we hit
+            // the OS limit of open files for the process - on Linux, where sockets count as files).
+            // Running builds where we limit this value all the way down to 100 didn't see
+            // any noticeable performance impact, so 30k shouldn't pose a problem.
+            // The configurable limit is per-client and per-server, but because we will reuse this HttpClient
+            // for all BlobClients, we are effectively limiting the number of open connections in general.
+            MaxConnectionsPerServer = 30_000
+        });
+
+    private static readonly HttpClientTransport Transport = new(HttpClient);
+
+    /// <summary>
+    /// Centralized point to override options for all blob clients created by the application.
+    /// </summary>
+    public static BlobClientOptions CreateOrOverride(BlobClientOptions? baseline)
+    {
+        baseline ??= new BlobClientOptions();
+        baseline.Transport = Transport;
+        return baseline;
+    }
+}

Public/Src/Cache/ContentStore/Interfaces/Auth/ManagedIdentityAzureStorageCredentials.cs

@@ -7,46 +7,45 @@
 using Azure.Storage.Blobs;
 using Azure.Storage.Blobs.ChangeFeed;
 
-namespace BuildXL.Cache.ContentStore.Interfaces.Auth
+namespace BuildXL.Cache.ContentStore.Interfaces.Auth;
+
+/// <nodoc />
+public class ManagedIdentityAzureStorageCredentials : IAzureStorageCredentials
 {
+    private static readonly Regex StorageAccountNameRegex = new("https://(?<accountName>[^\\.]+)\\.blob\\..*", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+
+    private readonly ManagedIdentityCredential _credentials;
+    private readonly Uri _blobUri;
+
     /// <nodoc />
-    public class ManagedIdentityAzureStorageCredentials : IAzureStorageCredentials
+    public ManagedIdentityAzureStorageCredentials(string managedIdentityClientId, Uri blobUri)
     {
-        private static readonly Regex StorageAccountNameRegex = new("https://(?<accountName>[^\\.]+)\\.blob\\..*", RegexOptions.Compiled | RegexOptions.IgnoreCase);
+        _credentials = new ManagedIdentityCredential(managedIdentityClientId);
+        _blobUri = blobUri;
+    }
 
-        private readonly ManagedIdentityCredential _credentials;
-        private readonly Uri _blobUri;
+    /// <inheritdoc />
+    public string GetAccountName()
+    {
+        var match = StorageAccountNameRegex.Match(_blobUri.ToString());
 
-        /// <nodoc />
-        public ManagedIdentityAzureStorageCredentials(string managedIdentityClientId, Uri blobUri)
+        if (match.Success)
         {
-            _credentials = new ManagedIdentityCredential(managedIdentityClientId);
-            _blobUri = blobUri;
+            return match.Groups["accountName"].Value;
         }
 
-        /// <inheritdoc />
-        public string GetAccountName()
-        {
-            var match = StorageAccountNameRegex.Match(_blobUri.ToString());
-
-            if (match.Success)
-            {
-                return match.Groups["accountName"].Value;
-            }
-
-            throw new InvalidOperationException($"The provided URI is malformed and the account name could not be retrieved.");
-        }
+        throw new InvalidOperationException($"The provided URI is malformed and the account name could not be retrieved.");
+    }
 
-        /// <inheritdoc />
-        public BlobServiceClient CreateBlobServiceClient(BlobClientOptions? blobClientOptions = null)
-            => new(_blobUri, _credentials, blobClientOptions ?? new(BlobClientOptions.ServiceVersion.V2021_02_12));
+    /// <inheritdoc />
+    public BlobServiceClient CreateBlobServiceClient(BlobClientOptions? blobClientOptions = null)
+        => new(_blobUri, _credentials, BlobClientOptionsFactory.CreateOrOverride(blobClientOptions));
 
-        /// <inheritdoc />
-        public BlobContainerClient CreateContainerClient(string containerName, BlobClientOptions? blobClientOptions = null)
-            => new(new Uri(_blobUri, containerName), _credentials, blobClientOptions ?? new(BlobClientOptions.ServiceVersion.V2021_02_12));
+    /// <inheritdoc />
+    public BlobContainerClient CreateContainerClient(string containerName, BlobClientOptions? blobClientOptions = null)
+        => new(new Uri(_blobUri, containerName), _credentials, BlobClientOptionsFactory.CreateOrOverride(blobClientOptions));
 
-        /// <inheritdoc />
-        public BlobChangeFeedClient CreateBlobChangeFeedClient(BlobClientOptions? blobClientOptions = null, BlobChangeFeedClientOptions? changeFeedClientOptions = null)
-            => new BlobChangeFeedClient(_blobUri, _credentials, blobClientOptions ?? new(BlobClientOptions.ServiceVersion.V2021_02_12), changeFeedClientOptions ?? new());
-    }
+    /// <inheritdoc />
+    public BlobChangeFeedClient CreateBlobChangeFeedClient(BlobClientOptions? blobClientOptions = null, BlobChangeFeedClientOptions? changeFeedClientOptions = null)
+        => new BlobChangeFeedClient(_blobUri, _credentials, BlobClientOptionsFactory.CreateOrOverride(blobClientOptions), changeFeedClientOptions ?? new());
 }