From eab9339c14ee267462103e2c971ee5cebeb09984 Mon Sep 17 00:00:00 2001 From: Kevin Cunnane Date: Wed, 11 Oct 2017 11:14:47 -0700 Subject: [PATCH] Improve performance of Create Firewall Rule code (#489) * Multi-thread server lookup to increase perf for multiple subscriptions * Use parallel execution for listsubscriptions - Also refactored parallel execution code to be a bit more generic --- .../Extensibility/IEnumerableExt.cs | 2 +- .../AzureDatabaseDiscoveryProvider.cs | 4 +- .../Discovery/AzureUtil.cs | 35 +++++++----- .../Firewall/FirewallRuleService.cs | 57 ++++++++++++++----- .../AzureResourceManager.cs | 54 ++++++++++++------ 5 files changed, 104 insertions(+), 48 deletions(-) diff --git a/src/Microsoft.SqlTools.Hosting/Extensibility/IEnumerableExt.cs b/src/Microsoft.SqlTools.Hosting/Extensibility/IEnumerableExt.cs index 5f332b24..309fbc1f 100644 --- a/src/Microsoft.SqlTools.Hosting/Extensibility/IEnumerableExt.cs +++ b/src/Microsoft.SqlTools.Hosting/Extensibility/IEnumerableExt.cs @@ -8,7 +8,7 @@ using System.Collections.Generic; namespace Microsoft.SqlTools.Extensibility { - internal static class IEnumerableExt + public static class IEnumerableExt { public static IEnumerable SingleItemAsEnumerable(this T item) { diff --git a/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureDatabaseDiscoveryProvider.cs b/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureDatabaseDiscoveryProvider.cs index 37ffb1ca..7c823e6c 100644 --- a/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureDatabaseDiscoveryProvider.cs +++ b/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureDatabaseDiscoveryProvider.cs @@ -121,7 +121,7 @@ namespace Microsoft.SqlTools.ResourceProvider.Core IEnumerable subscriptions = await GetSubscriptionsAsync(string.IsNullOrEmpty(serverName)); if (!cancellationToken.IsCancellationRequested) { - result = await AzureUtil.ExecuteGetAzureResourceAsParallel(null, subscriptions, serverName, cancellationToken, + result = await AzureUtil.ExecuteGetAzureResourceAsParallel((object)null, subscriptions, serverName, cancellationToken, GetDatabaseForSubscriptionAsync); } @@ -215,7 +215,7 @@ namespace Microsoft.SqlTools.ResourceProvider.Core /// /// Returns a list of Azure sql databases for given subscription /// - private async Task> GetDatabaseForSubscriptionAsync(IAzureResourceManagementSession parentSession, + private async Task> GetDatabaseForSubscriptionAsync(object notRequired, IAzureUserAccountSubscriptionContext input, string serverName, CancellationToken cancellationToken, CancellationToken internalCancellationToken) { diff --git a/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureUtil.cs b/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureUtil.cs index abde06a0..3d7324ec 100644 --- a/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureUtil.cs +++ b/src/Microsoft.SqlTools.ResourceProvider.Core/Discovery/AzureUtil.cs @@ -10,25 +10,30 @@ using System.Threading.Tasks; namespace Microsoft.SqlTools.ResourceProvider.Core { - internal static class AzureUtil + /// + /// Utils class supporting parallel querying of Azure resources + /// + public static class AzureUtil { /// /// Execute an async action for each input in the a list of input in parallel. /// If any task fails, adds the exeption message to the response errors - /// If cancellation token is set to cancel, returns empty response - /// + /// If cancellation token is set to cancel, returns empty response. + /// Note: Will not throw if errors occur. Instead the caller should check for errors and either notify + /// or rethrow as needed. + /// /// Resource management session to use to call the resource manager /// List of inputs - /// server name to filter the result + /// optional lookup key to filter the result /// Cancellation token /// Async action /// ServiceResponse including the list of data and errors - public static async Task> ExecuteGetAzureResourceAsParallel( - IAzureResourceManagementSession session, + public static async Task> ExecuteGetAzureResourceAsParallel( + TConfig config, IEnumerable inputs, - string serverName, + string lookupKey, CancellationToken cancellationToken, - Func { - ServiceResponse result = await GetResult(session, inputList[i], serverName, cancellationToken, + ServiceResponse result = await GetResult(config, inputList[i], lookupKey, cancellationToken, cancellationTokenSource.Token, asyncAction); //server name is used to filter the result and if the data is already found not, we need to cancel the other tasks - if (!string.IsNullOrEmpty(serverName) && result.Found) + if (!string.IsNullOrEmpty(lookupKey) && result.Found) { cancellationTokenSource.Cancel(); } @@ -82,13 +87,13 @@ namespace Microsoft.SqlTools.ResourceProvider.Core return new ServiceResponse(mergedResult, mergedErrors); } - private static async Task> GetResult( - IAzureResourceManagementSession session, + private static async Task> GetResult( + TConfig config, TInput input, - string serverName, + string lookupKey, CancellationToken cancellationToken, CancellationToken internalCancellationToken, - Func response = await AzureUtil.ExecuteGetAzureResourceAsParallel((object)null, + subscriptions, serverName, new CancellationToken(), TryFindAzureResourceForSubscriptionAsync); + + if (response != null) { - using (IAzureResourceManagementSession session = await ResourceManager.CreateSessionAsync(subscription)) + if (response.Data != null && response.Data.Any()) { - IAzureSqlServerResource azureSqlServer = await FindAzureResourceForSubscriptionAsync(serverName, session); - - if (azureSqlServer != null) - { - return new FirewallRuleResource() - { - SubscriptionContext = subscription, - AzureResource = azureSqlServer - }; - } + return response.Data.First(); + } + if (response.HasError) + { + var error = response.Errors.FirstOrDefault(); + throw new FirewallRuleException(error.Message, error); } } + // Else throw as we couldn't find the resource as expected var currentUser = await AuthenticationManager.GetCurrentAccountAsync(); throw new FirewallRuleException(string.Format(CultureInfo.CurrentCulture, SR.AzureServerNotFound, serverName, currentUser != null ? currentUser.UniqueId : string.Empty)); @@ -187,6 +190,34 @@ namespace Microsoft.SqlTools.ResourceProvider.Core.Firewall throw new FirewallRuleException(SR.FirewallRuleCreationFailed, ex); } } + + /// + /// Returns a list of Azure sql databases for given subscription + /// + private async Task> TryFindAzureResourceForSubscriptionAsync(object notRequired, + IAzureUserAccountSubscriptionContext input, string serverName, + CancellationToken cancellationToken, CancellationToken internalCancellationToken) + { + ServiceResponse result = null; + if (!cancellationToken.IsCancellationRequested) + { + using (IAzureResourceManagementSession session = await ResourceManager.CreateSessionAsync(input)) + { + IAzureSqlServerResource azureSqlServer = await FindAzureResourceForSubscriptionAsync(serverName, session); + if (azureSqlServer != null) + { + result = new ServiceResponse(new FirewallRuleResource() + { + SubscriptionContext = input, + AzureResource = azureSqlServer + }.SingleItemAsEnumerable()); + result.Found = true; + } + } + } + + return result ?? new ServiceResponse(); + } /// /// Throws a firewallRule exception based on give status code diff --git a/src/Microsoft.SqlTools.ResourceProvider.DefaultImpl/AzureResourceManager.cs b/src/Microsoft.SqlTools.ResourceProvider.DefaultImpl/AzureResourceManager.cs index b810bb73..06e5a8bd 100644 --- a/src/Microsoft.SqlTools.ResourceProvider.DefaultImpl/AzureResourceManager.cs +++ b/src/Microsoft.SqlTools.ResourceProvider.DefaultImpl/AzureResourceManager.cs @@ -21,6 +21,7 @@ using System.Globalization; using Microsoft.Rest.Azure; using Microsoft.SqlTools.ResourceProvider.Core; using System.Collections; +using System.Threading; namespace Microsoft.SqlTools.ResourceProvider.DefaultImpl { @@ -269,27 +270,46 @@ namespace Microsoft.SqlTools.ResourceProvider.DefaultImpl public async Task> GetSubscriptionContextsAsync(IAzureUserAccount userAccount) { List contexts = new List(); - foreach (IAzureTenant tenant in userAccount.AllTenants) + Stopwatch stopwatch = new Stopwatch(); + stopwatch.Start(); + ServiceResponse response = await AzureUtil.ExecuteGetAzureResourceAsParallel( + userAccount, userAccount.AllTenants, string.Empty, CancellationToken.None, GetSubscriptionsForTentantAsync); + + if (response.HasError) { - AzureTenant azureTenant = tenant as AzureTenant; - if (azureTenant != null) - { - ServiceClientCredentials credentials = CreateCredentials(azureTenant); - using (SubscriptionClient client = new SubscriptionClient(_resourceManagementUri, credentials)) - { - IEnumerable subs = await GetSubscriptionsAsync(client); - contexts.AddRange(subs.Select(sub => - { - AzureSubscriptionIdentifier subId = new AzureSubscriptionIdentifier(userAccount, azureTenant.TenantId, sub.SubscriptionId, _resourceManagementUri); - AzureUserAccountSubscriptionContext context = new AzureUserAccountSubscriptionContext(subId, credentials); - return context; - })); - } - } + var ex = response.Errors.First(); + throw new AzureResourceFailedException( + string.Format(CultureInfo.CurrentCulture, SR.AzureSubscriptionFailedErrorMessage, ex.Message)); } + contexts.AddRange(response.Data); + stopwatch.Stop(); + TraceEvent(TraceEventType.Verbose, (int)TraceId.AzureResource, "Time taken to get all subscriptions was {0}ms", stopwatch.ElapsedMilliseconds.ToString()); return contexts; } - + + private async Task> GetSubscriptionsForTentantAsync( + IAzureUserAccount userAccount, IAzureTenant tenant, string lookupKey, + CancellationToken cancellationToken, CancellationToken internalCancellationToken) + { + + AzureTenant azureTenant = tenant as AzureTenant; + if (azureTenant != null) + { + ServiceClientCredentials credentials = CreateCredentials(azureTenant); + using (SubscriptionClient client = new SubscriptionClient(_resourceManagementUri, credentials)) + { + IEnumerable subs = await GetSubscriptionsAsync(client); + return new ServiceResponse(subs.Select(sub => + { + AzureSubscriptionIdentifier subId = new AzureSubscriptionIdentifier(userAccount, azureTenant.TenantId, sub.SubscriptionId, _resourceManagementUri); + AzureUserAccountSubscriptionContext context = new AzureUserAccountSubscriptionContext(subId, credentials); + return context; + })); + } + } + return new ServiceResponse(); + } + /// /// Returns the azure resource groups for given subscription ///