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: remove retained resources logics for non State-of-The-World resources #9724

Merged
merged 2 commits into from
Dec 2, 2022

Conversation

YifeiZhuang
Copy link
Contributor

I argue there was never a reason for it.
The perceived value was that the dependent resources would be needed. That is true, but they would already be retained by the components outside of xds client. For example, the xds resolver watches the dependent RDS resources that were referenced in LDS.
-ejona@

follow up on #9444
Brief description document of the motivation: @ejona86 it would be great if you can have a glance on it. https://docs.google.com/document/d/18SYwBEt4vLsfKiJ_XOOo8lgi_HTovjz7VEhmCDkmmF4/edit#heading=h.h2ddie7y1g08

@YifeiZhuang YifeiZhuang requested a review from ejona86 December 1, 2022 00:43
// Non-null for State of the World resources.
@Nullable
abstract XdsResourceType<?> dependentResource();
abstract boolean isStateOfTheWorld();
Copy link
Member

Choose a reason for hiding this comment

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

As silly as it seems, Envoy uses the "state of the world" terminology for all resource types when using the non-delta protocol (which is what we are doing). SotW is an aspect of the RPC method and not the resource type. It's just that some resource types behave differently in SotW from each other...

https://www.envoyproxy.io/docs/envoy/latest/api-docs/xds_protocol

The SotW approach was the original mechanism used by xDS, in which the client must specify all resource names it is interested in with each request, and for LDS and CDS resources, the server must return all resources that the client has subscribed to in each request.

So we should use a different word for this method. I don't know what to name it though. "isFullStateOfTheWorld"? Throw a short javadoc on the method to resolve any source of ambiguity.

@YifeiZhuang YifeiZhuang merged commit c87fc05 into grpc:master Dec 2, 2022
@YifeiZhuang YifeiZhuang deleted the dependent-resources branch December 2, 2022 18:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 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.

2 participants