Azure / AppConfiguration-DotnetProvider

The .NET Standard configuration provider for Azure App Configuration
https://github.com/Azure/AppConfiguration
MIT License
83 stars 37 forks source link

Use time-based retry for startup #458

Closed zhenlan closed 10 months ago

zhenlan commented 1 year ago

Problem

The provider uses count-based retry today. If an App Configuration store experiences transient errors (eg., momentary throttling), the provider may give up too quickly with a couple of retries (may only take a few milliseconds or seconds). This can cause applications to fail to start and blackout.

Proposal

The proposal is to use a time-based retry logic for the startup. We may allow the retry time to be customized, but the provider should have a default value that is reasonably long enough, for example, 1 minute. This way, the provider won't give up too quickly. Note:

cc @jimmyca15 @drago-draganov @avanigupta

amerjusupovic commented 1 year ago
  • This should include calls to the Key Vault too (for Key Vault reference)

What should the behavior be in the case where we are accessing a Key Vault for the first time during refresh, not on provider startup? Would we expect to keep track of the first time each key vault is accessed?

zhenlan commented 1 year ago

Just to make sure I understand the question, why does it not matter if a Key Vault is accessed for the first-time during refresh?

amerjusupovic commented 1 year ago

I might have misunderstood the issue description, but I read it as applying startup retry logic to all Key Vault reads, not just during startup. So yes, I was wondering if we should treat accessing a Key Vault for the first time (even during refresh) like accessing an App Config store for the first time, but now I think that doesn't sound necessary.

zhenlan commented 1 year ago

Right, no change to refresh for either App Config or Key Vault.