-
Notifications
You must be signed in to change notification settings - Fork 40
Fix bug with endpoint failover #680
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR fixes how endpoint failover backoff is applied by ensuring all clients back off after a non-failoverable exception and adds a test validating this behavior.
- Add a unit test to verify all clients back off after a non-failoverable exception during refresh.
- Fix backoff logic to use the current client’s endpoint rather than the previous one.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Tests.AzureAppConfiguration/Unit/FailoverTests.cs | New test FailOverTests_AllClientsBackedOffAfterNonFailoverableException added |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs | Changed backoff status update to use GetEndpointForClient(currentClient) instead of previousEndpoint |
src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs
Show resolved
Hide resolved
|
@microsoft-github-policy-service agree |
|
@microsoft-github-policy-service agree company="Microsoft" |
|
CLA cannot be agreed. Move to #686 |
Currently, the provider doesn't backoff all replicas as expected.
Expected
If there are 2 or more endpoints (1 store endpoint and 1+ replicas) and a request to the first endpoint fails with an exception that the provider does not consider "failoverable" (requesting another endpoint will not solve the issue), all endpoints will be backed off with an exponential backoff factoring in 1 failed request. This means the initial backoff will be around the minimum backoff time of 30 seconds.
Actual
If there are 2 or more endpoints (1 store endpoint and 1+ replicas) and a request to the first endpoint fails with an exception that the provider does not consider "failoverable" (requesting another endpoint will not solve the issue), only the current endpoint will be backed off. The backoff is also incorrectly calculated to be longer than expected.