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

rls: Guarantee backoff will update RLS picker #11202

Merged
merged 1 commit into from
May 13, 2024

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented May 13, 2024

Previously, picker was likely null if entering backoff soon after start-up. This prevented the picker from being updated and directing queued RPCs to the fallback. It would work for new RPCs if RLS returned extremely rapidly; both ManagedChannelImpl and DelayedClientTransport do a pick before enqueuing so the ManagedChannelImpl pick could request from RLS and DelayedClientTransport could use the response. So the test uses a delay to purposefully avoid that unlikely-in-real-life case.

Creating a resolving OOB channel for InProcess doesn't actually change the destination from the parent, because InProcess uses directaddress. Thus the fakeRlsServiceImpl is now being added to the fake backend server, because the same server is used for RLS within the test.

b/333185213

Previously, picker was likely null if entering backoff soon after
start-up. This prevented the picker from being updated and directing
queued RPCs to the fallback. It would work for new RPCs if RLS returned
extremely rapidly; both ManagedChannelImpl and DelayedClientTransport do
a pick before enqueuing so the ManagedChannelImpl pick could request
from RLS and DelayedClientTransport could use the response. So the test
uses a delay to purposefully avoid that unlikely-in-real-life case.

Creating a resolving OOB channel for InProcess doesn't actually change
the destination from the parent, because InProcess uses directaddress.
Thus the fakeRlsServiceImpl is now being added to the fake backend
server, because the same server is used for RLS within the test.

b/333185213
@ejona86 ejona86 requested a review from larry-safran May 13, 2024 14:22
@ejona86 ejona86 added TODO:backport PR needs to be backported. Removed after backport complete TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved labels May 13, 2024
@ejona86 ejona86 merged commit f9b6e5f into grpc:master May 13, 2024
15 checks passed
@ejona86 ejona86 deleted the rls-fallback-hang branch May 13, 2024 23:29
@ejona86 ejona86 removed TODO:backport PR needs to be backported. Removed after backport complete TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved labels May 14, 2024
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 this pull request may close these issues.

None yet

2 participants