Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use time-based retry for startup #458

Closed
zhenlan opened this issue Sep 12, 2023 · 4 comments · Fixed by #488
Closed

Use time-based retry for startup #458

zhenlan opened this issue Sep 12, 2023 · 4 comments · Fixed by #488
Assignees

Comments

@zhenlan
Copy link
Contributor

zhenlan commented Sep 12, 2023

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:

  • This is for startup only. We don't need to do this for refresh. The refresh happens periodically. We don't want a refresh to last too long.
  • This should include calls to the Key Vault too (for Key Vault reference)

cc @jimmyca15 @drago-draganov @avanigupta

@amerjusupovic
Copy link
Member

  • 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
Copy link
Contributor Author

zhenlan commented Sep 25, 2023

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
Copy link
Member

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
Copy link
Contributor Author

zhenlan commented Sep 26, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants