Resolve key vault references concurrently

[linglingye001]

This PR introduces an opt-in capability to resolve Azure Key Vault references concurrently during Azure App Configuration load, aiming to reduce startup time when many Key Vault references are present. #735

Changes:

• Add ParallelSecretResolutionEnabled option under Key Vault configuration and plumb it into provider options.
• Update configuration loading to optionally process adapter resolution concurrently.
• Add unit tests covering parallel resolution behavior and default sequential behavior

[Copilot]

Pull request overview

This PR introduces an opt-in capability to resolve Azure Key Vault references concurrently during Azure App Configuration load, aiming to reduce startup time when many Key Vault references are present.

Changes:

• Add ParallelSecretResolutionEnabled option under Key Vault configuration and plumb it into provider options.
• Update configuration loading to optionally process adapter resolution concurrently and merge results deterministically.
• Add unit tests covering parallel resolution behavior and default sequential behavior; add locking in the Key Vault secret provider to support concurrent access.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/Tests.AzureAppConfiguration/Unit/KeyVaultReferenceTests.cs	Adds tests validating parallel Key Vault resolution and default sequential behavior.
src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs	Adds synchronization around secret caching and refresh bookkeeping for thread safety under concurrency.
src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs	Adds parallel adapter processing path and factors merge logic into a helper.
src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs	Stores whether parallel secret resolution is enabled after Key Vault configuration.
src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs	Introduces the public ParallelSecretResolutionEnabled toggle with documentation.

Comments suppressed due to low confidence (1)

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs:654

• In parallel mode, all adapter tasks are dispatched before any failures are observed (via Task.WhenAll). If a single Key Vault reference is invalid/unavailable, the load will still issue requests for the remaining references, increasing latency and side effects (extra Key Vault traffic) even though the overall load fails. Consider failing fast by cancelling remaining work when the first task faults (e.g., linked CancellationTokenSource + cancel on first exception, or processing in bounded batches).

                // Dispatch adapter processing for all settings concurrently. Only Key Vault references
                // perform network I/O during adapter processing; other adapters complete synchronously.
                // Insertion order in 'data' is preserved when merging results so prefix-stripping and
                // last-write-wins behavior remain unchanged.
                var pendingTasks = new List<Task<IEnumerable<KeyValuePair<string, string>>>>(data.Count);

                foreach (KeyValuePair<string, ConfigurationSetting> kvp in data)
                {
                    if (_requestTracingEnabled && _requestTracingOptions != null)
                    {
                        _requestTracingOptions.UpdateAiConfigurationTracing(kvp.Value.ContentType);
                    }

                    pendingTasks.Add(ProcessAdapters(kvp.Value, cancellationToken));
                }

                IEnumerable<KeyValuePair<string, string>>[] results = await Task.WhenAll(pendingTasks).ConfigureAwait(false);

                for (int i = 0; i < results.Length; i++)
                {
                    MergeIntoApplicationData(applicationData, results[i]);
                }
            }

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

[zhiyuanliang-ms]

The reason we remove the _nextRefreshSourceId and _nextRefreshTime is to simplify the implementation. Otherwise, we have to add lock here, because they will be updated in the GetSecretValue code path (the SetSecretInCache call)

The benefit of maintaining the _netRefresh pair is that the ShouldRefreshKeyVaultSecrets call will be an O(1) operation. The implementation in this PR makes it an O(n) operation. But I think there won't be too many cached secrets and iterate the whole concurrent dictionary is not that expensive. So I think it is fine.

cc @avanigupta @jimmyca15

[avanigupta]

This change breaks config composition when multiple settings share the same key. Key Vault references are now always appended at the end, causing them to override existing key-values and effectively ignore the selection precedence order. We need to avoid this behavior.

[zhiyuanliang-ms]

Are loading configuration settings selector by selector? My understanding is that under one selector, there will not be duplicated key name, so the override behavior only happens across selectors.

[avanigupta]

That is correct. We set the order during load and need to maintain the same precedence order during adapter processing.

[avanigupta]

I'm thinking of a different approach for this PR. What if we added a new method to the IKeyValueAdapter interface which pre-loads data before ProcessKeyValue is called. The pre-loader would be responsible for parallelization of KeyVault calls, and it would update the secret cache so that by the time ProcessKeyValue is invoked, it just reads from cache (no I/O in ProcessKeyValue). This would preserve the precedence order for config composition. Other adapters dont have anything to preload, so they are no-op.

Task PreloadAsync(IEnumerable<ConfigurationSetting> settings, Logger logger, CancellationToken cancellationToken);

Semantics: "Given all the settings about to be processed, pre-warm your caches so that ProcessKeyValue can return instantly."

[avanigupta]

We need to define a max concurrency limit.

[linglingye001]

how about 16? do you have any suggestion?

[linglingye001]

looks like there's no recommended number from the doc https://learn.microsoft.com/en-us/azure/key-vault/general/overview-throttling#how-to-throttle-your-app-in-response-to-service-limits

[avanigupta]

KeyVault has a limit of 4000 requests per 10 seconds per vault: https://learn.microsoft.com/en-us/azure/key-vault/general/service-limits#secrets-managed-storage-account-keys-and-vault-transactions

What if we made the concurrency configurable by user?
Instead of exposing a boolean option to turn on/off parallel secret resolution, we could offer something like int MaxSecretResolutionConcurrency. The default value will be 1 (i.e., sequential load from KeyVault). Anything higher than 1 will enable parallel secret resolution.
Users can configure it to whatever max concurrency they want based on their Key Vault usage.

cc @jimmyca15

[zhiyuanliang-ms]

This topic has been discussed for JS provider. See comment

I think there are several things we should align:

1. For server side scenario, it will be very difficult to hit the key vault maximum transactions. If there are 100 secrets in a store, there must be more than 40 instances start/refresh within 10 seconds.
2. For the most critical configuration initial load path, we have startup retry on. So even we are throttled, it will be retried.
3. The customer said he only had 10+ secret references.

I think instead of managing the maximum concurrency by ourselves, we should take advantage of key vault sdk's built-in retry mechanism. For those uses who use secret reference very heavily, they should pass secret client instance directly with retry policy configured in secret client options.

[zhiyuanliang-ms]

Discussed offline with @jimmyca15

We aligned on that the purpose of limit the max concurrency is not to avoid key vault throttling, but to be responsible for memory usage.

If there are thousands of secret references and we create a task for each one, memory usage could explode.

Therefore, instead of exposing a configurable MaxSecretResolutionConcurrency option, we just need to enforce an internal concurrency limit to ensure that parallel secret resolution uses a manageable amount of memory.

[avanigupta]

Btw, there is precedent for other Azure services exposing the concurrency limit in parallel resolution scenarios like these.
For eg:

• StorageTransferOptions.MaximumConcurrency
• ServiceBusProcessorOptions.MaxConcurrentCalls
• LogsUploadOptions.MaxConcurrency

We can always define our upper limit for this config. But if we already exposed boolean option in JS provider, I'm good with keeping boolean option here too. If we have to keep it internal, 16 sounds like a good limit to me.