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

xds: deletion only to watchers of same control plane #9896

Merged
merged 10 commits into from
Feb 17, 2023

Conversation

temawi
Copy link
Contributor

@temawi temawi commented Feb 16, 2023

When XdsClient learns that a control plane no longer tracks a resource, it should only notify watchers associated with that control plane.

This matters in control plane federation cases when more than one control plane is in use.

When XdsClient learns that a control plane no longer tracks a resource,
it should only notify watchers associated with that control plane.

This matters in control plane federation cases when more than one
control plane is in use.
@temawi temawi requested a review from ejona86 February 16, 2023 16:07
@ejona86 ejona86 requested a review from YifeiZhuang February 16, 2023 16:37
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test takes 12.5 s on my machine. That isn't the deletion detection timer. It looks to be the control plane shutdown awaitTermination. Because you neither unregister the watches not shutdown the xdsClient, xdsClient still has an ADS stream to the fake control planes so you pay 2*5 seconds. The remaining 2.5s may just be classloading.

@temawi temawi requested a review from ejona86 February 16, 2023 19:18
Copy link
Contributor

@YifeiZhuang YifeiZhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know that the two control planes will never use the same resource names among all resource types? To allow for it, we ultimately need to have different subscribers for the same name but different control planes.

@ejona86
Copy link
Member

ejona86 commented Feb 16, 2023

Do we know that the two control planes will never use the same resource names among all resource types?

Well, we are the one that requests resources by names from a particular control plane. So the initial resources (listeners) would definitely have to have different xdstp authorities, otherwise we'd just talk to one control plane. When a listener references other resources, we use the authority of that resource which is totally allowed to be a different server. That allows servers to delegate to one other.

@temawi temawi merged commit b481f34 into grpc:master Feb 17, 2023
@temawi temawi deleted the federation-deletion branch February 17, 2023 14:34
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 19, 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.

3 participants