Skip to content

rls: Refresh name resolution on rejected addresses #10032

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

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

temawi
Copy link
Contributor

@temawi temawi commented Apr 10, 2023

If a child load balancer rejects the addresses it if given all we can do is to trigger a name resolution refresh and hope for a better set of addresses.

If a child load balancer rejects the addresses it if given all we can do
is to trigger a name resolution refresh and hope for a better set of
addresses.
@temawi temawi requested a review from larry-safran April 10, 2023 21:01
@temawi
Copy link
Contributor Author

temawi commented Apr 10, 2023

@ejona86 We discussed this, but I'll still ask: what prevents this change in resulting in a loop where the name resolver keeps providing addresses the child LB does not like and us repeatedly refreshing name resolution?

@larry-safran
Copy link
Contributor

It is reasonable to expect that at some time in the future name resolution might start returning something valid, but it seems like we ought to have exponential backoff with max tries, after which we generate an error.

@ejona86
Copy link
Member

ejona86 commented Apr 10, 2023

what prevents this change in resulting in a loop where the name resolver keeps providing addresses the child LB does not like and us repeatedly refreshing name resolution?

Because we aren't calling refresh when the NR provides new addresses. When the NR provides new addresses, we should return false instead. (Said another way, acceptResolvedAddresses() and refresh() will not be in the same callstack)

@temawi temawi merged commit 6e54ceb into grpc:master Apr 14, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants