xds: allow "*" to indicate a wildcard subscription #16861

markdroth · 2021-06-07T21:33:08Z

Signed-off-by: Mark D. Roth [email protected]

Commit Message: xds: allow * to indicate a wildcard subscription
Additional Description: Update xDS spec to say that * indicates a wildcard subscription and the existing behavior of specifying no resource names to subscribe to is just a syntactic alias for subscribing to *.
Risk Level: Low
Testing: N/A
Docs Changes: Included in PR
Release Notes: N/A
Platform Specific Features: N/A

Alternative doc proposal for #16855. CC @krnowak

Signed-off-by: Mark D. Roth <[email protected]>

dmitri-d · 2021-06-07T22:58:38Z

docs/root/api-docs/xds_protocol.rst

-The incremental protocol also provides a mechanism for lazy loading of resources. For details on
-the incremental protocol, see :ref:`Incremental xDS <xds_protocol_delta>` below.
+interested in with each request, and the server must return all resources the client has
+subscribed to in each request (in LDS/CDS). This means that if the client is already subscribing


I think (in LDS/CDS) part makes the sentence confusing. Perhaps it's worth clarifying that in LDS/CDS there could be multiple resources in a reply, while for RDS/EDS it's always one?

To be clear, even in RDS and EDS, it's totally valid to have multiple resources per reply; for example, if the client is subscribed to 5 EDS resources and 2 of those resources change, the server can send a response including those two resources. The thing that's special about LDS and CDS is that the server is required to send all resources that the client is subscribed to in every response, even the ones that haven't changed. So if a client is subscribed to 5 LDS resources and 2 of them change, the server must send a response that includes all 5 resources. (This is because the SotW protocol doesn't provide a way for the server to explicitly indicate to the client that a resource has been removed, so the only way to remove an LDS or CDS resource is by not including it in a response. And the removal criteria for RDS and EDS resources is even messier; see the "Deleting Resources" section below.)

Anyway, I've attempted to reword this a bit to clarify it, but I haven't changed the phraseology about the condition this is discussing, because that was already correct.

Thanks for the rewording, I think it's much clearer now.

To be clear, even in RDS and EDS, it's totally valid to have multiple resources per reply

Hmm, not according to the implementation (rds: https://github.com/envoyproxy/envoy/blob/main/source/common/router/rds_impl.cc#L227, eds: https://github.com/envoyproxy/envoy/blob/main/source/common/upstream/eds.cc#L166): updates with more than resource will be dropped.

I believe that Envoy currently creates a separate stream for each resource when using non-aggregated EDS, as per #2943, and the same may be true of RDS. However, this is a limitation of Envoy's implementation, not of the xDS protocol. There is no protocol-level reason why there cannot be more than one resource send per response for any resource type.

Note that when using ADS, Envoy does support multiple resources per response for EDS and RDS. But nothing about that functionality is actually tied to ADS; the exact same thing could be done in non-aggregated streams.

dmitri-d · 2021-06-07T22:59:09Z

docs/root/api-docs/xds_protocol.rst

+subscribed to in each request (in LDS/CDS). This means that if the client is already subscribing
+to 99 resources and wants to add an additional one, it must send a request with all 100 resource
+names, rather than just the one new one. And the server must then respond by sending all 100
+resources, even if the 99 that were already subscribed to have not changed (in LDS/CDS). This


ditto here re: (in LDS/CDS).

Updated wording here too.

dmitri-d · 2021-06-07T23:00:16Z

lgtm, other than a small nit: re LDS/CDS clarification.

krnowak · 2021-06-08T11:00:47Z

The wording sounds better than mine, thanks. Should the change get an entry in docs/root/version_history/current.rst?

Signed-off-by: Mark D. Roth <[email protected]>

markdroth · 2021-06-08T15:44:36Z

The wording sounds better than mine, thanks. Should the change get an entry in docs/root/version_history/current.rst?

I don't think it needs one, since it's not an implementation change, just a spec change.

krnowak

LGTM.

dmitri-d · 2021-06-08T19:45:26Z

lgtm.

htuch

Awesome, sorry for the delayed review.
/wait

htuch · 2021-06-09T23:21:12Z

docs/root/api-docs/xds_protocol.rst

+
+For historical reasons, if the initial request on the stream for a given resource type contains no
+resource names, the server should treat that as a subscription to "*". A client may later
+unsubscribe to "*", or it may later add a subscription to an additional resource name.


Note that we are going to break any existing setup that names a singleton resource *. Seems a bit far fetched, but technically a breaking change. If we instead used something like xds_internal://* here as a name (or something unlikely to collide), we might stand a lower chance of collision but don't feel super strong on this one.

I think that no matter what name we pick here, collisions are possible. I'd rather keep the name short and intuitive.

htuch · 2021-06-09T23:24:14Z

docs/root/api-docs/xds_protocol.rst

-that once a stream has entered wildcard mode for a given resource type, there is no way to change
-the stream out of wildcard mode; resource names specified in any subsequent request on the stream
-will be ignored.
+types, there is also a "wildcard" subscription, which is triggered when subscribing to the special


Do we cover how * and on-demand work together in a SotW request in this spec?

Also how non-explicit * and explicit * subscriptions can be converted or used interchangeably? E.g. can I subscribe to {} but unsubscribe from *?

This change isn't really about on-demand; it's just about how to represent a wildcard subscription as a discrete name that can coexist with other subscriptions for the same resource type on the same stream. This mechanism is a prerequisite for on-demand CDS, but it's not actually part of the same feature.

I've made some changes in the next paragraph to attempt to clarify the semantics, and I've added some examples to help illustrate.

docs/root/api-docs/xds_protocol.rst

Signed-off-by: Mark D. Roth <[email protected]>

htuch

LGTM; needs CI docs fix though. Thanks!

Signed-off-by: Mark D. Roth <[email protected]>

markdroth · 2021-06-11T19:08:02Z

The CI failure looks unrelated to this PR. Is there a way to retry it?

dmitri-d · 2021-06-11T20:24:29Z

/retest

repokitteh-read-only · 2021-06-11T20:24:33Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16861 (comment) was created by @dmitri-d.

see: more, trace.

htuch

LGTM, thanks!

stevenzzzz · 2021-09-22T16:08:15Z

docs/root/api-docs/xds_protocol.rst

+explicitly subscribed to "*". However, once the client does explicitly subscribe to a resource
+name (whether it be "*" or any other name), then this legacy semantic is no longer available; at
+that point, clearing the list of subscribed resources is interpretted as an unsubscription (see
+:ref:`Unsubscribing From Resources<xds_protocol_unsubscribing>`) rather than as a subscription


there is a case this is not true:

client subscribes A, B, C

client unsubs A, B, C, connection broke, reconnection happens

on reconnection, client send empty init_resource_versions and subscribes, it's treated as wildcard client now.

If the client is no longer subscribed to any resources of a given resource type, then it will not send any request for that resource type when it reconnects, so that's not an issue.

stevenzzzz · 2021-09-22T16:14:31Z

docs/root/api-docs/xds_protocol.rst

+- Client sends a request with :ref:`resource_names_subscribe <envoy_v3_api_field_service.discovery.v3.DeltaDiscoveryRequest.resource_names_subscribe>` unset. Server interprets this as a subscription to "*".
+- Client sends a request with :ref:`resource_names_subscribe <envoy_v3_api_field_service.discovery.v3.DeltaDiscoveryRequest.resource_names_subscribe>` set to "A". Server interprets this as continuing the existing subscription to "*" and adding a new subscription to "A".
+- Client sends a request with :ref:`resource_names_unsubscribe <envoy_v3_api_field_service.discovery.v3.DeltaDiscoveryRequest.resource_names_unsubscribe>` set to "*". Server interprets this as unsubscribing to "*" and continuing the existing subscription to "A".
+- Client sends a request with :ref:`resource_names_unsubscribe <envoy_v3_api_field_service.discovery.v3.DeltaDiscoveryRequest.resource_names_unsubscribe>` set to "A". Server interprets this as unsubscribing to "A" (i.e., the client has now unsubscribed to all resources). Although the set of subscribed resources is now empty, just as it was after the initial request, it is not interpreted as a wildcard subscription, because there has previously been a request on this stream for this resource type that set the :ref:`resource_names_subscribe <envoy_v3_api_field_service.discovery.v3.DeltaDiscoveryRequest.resource_names_subscribe>` field.


reconnection will make things messy here if we dont explicitly say it's wildcard or not.

Could you clarify on this more?
Consider this case:

Client subscribes: A, *

Server sends: A, B, C, D

Client removes: *

Server has update for B, C, -D, and new resource E, now does it send (update B, C, remove D) to the client or not?

Also noteworthy that reconnection for the above scenario would become tricky:

reconnect happens, now client has A, B, C, D in the intial_resource_versions, Server will keep sending updates for A, B, C, D to the client.

reconnection will make things messy here if we dont explicitly say it's wildcard or not.

Could you clarify on this more?
Consider this case:

1. Client subscribes: A, * 2. Server sends: A, B, C, D 3. Client removes: * 4. Server has update for B, C, -D, and new resource E, now does it send (update B, C, remove D) to the client or not?

I'm not sure. I would say that either "server sends (remove B, C and D)" or "server sends (remove *)" and client takes care of dropping wildcard resources from its cache.

Also noteworthy that reconnection for the above scenario would become tricky:

1. reconnect happens, now client has A, B, C, D in the intial_resource_versions, Server will keep sending updates for A, B, C, D to the client.

I'd say that once client unsubscribes from wildcard, it should drop the wildcard resources it has, so if reconnect happens, only A should be in initial resources.

Consider this case:

Client subscribes: A, *

Server sends: A, B, C, D

Client removes: *

Server has update for B, C, -D, and new resource E, now does it send (update B, C, remove D) to the client or not?

As per the clarification I added in #17983, the server does not need to send anything for B, C, D, or E in this case.

Also noteworthy that reconnection for the above scenario would become tricky:

reconnect happens, now client has A, B, C, D in the intial_resource_versions, Server will keep sending updates for A, B, C, D to the client.

Once the client reconnects, it sends only A in resource_names_subscribe, so this is not an issue. Even if the other resources were somehow present in initial_resource_versions, that field does not affect the set of resources that the client is actually subscribed to.

stevenzzzz · 2021-09-22T16:17:27Z

docs/root/api-docs/xds_protocol.rst

-subscribed to is determined by the server instead of the client, so there is no mechanism
-for the client to unsubscribe from resources.
+subscribed to is determined by the server instead of the client, so the client cannot unsubscribe
+from those resources individually; it can only unsubscribe from the wildcard as a whole.


could you elaborate how to effectively unsubscribe resources that are "included " in the "wildcard subscription" when the client unsubscribes "*"?

I'd say that the client should be more or less able to track which resources are from wildcard subscription, so it should be able to drop them from its caches when unsubscribing from *.

Yes, @krnowak is correct. The client already knows which specific non-wildcard resources it is subscribed to, so when it unsubscribes from *, it can immediately drop all other resources from its cache.

Update xDS spec to say that * indicates a wildcard subscription and the existing behavior of specifying no resource names to subscribe to is just a syntactic alias for subscribing to *. Signed-off-by: Mark D. Roth <[email protected]>

xds: allow "*" to indicate a wildcard subscription

b956ea7

Signed-off-by: Mark D. Roth <[email protected]>

markdroth mentioned this pull request Jun 7, 2021

Add support for explicit wildcard resource #16855

Merged

dmitri-d reviewed Jun 7, 2021

View reviewed changes

clarify wording about LDS and CDS

4cac288

Signed-off-by: Mark D. Roth <[email protected]>

krnowak previously approved these changes Jun 8, 2021

View reviewed changes

dmitri-d previously approved these changes Jun 8, 2021

View reviewed changes

zuercher assigned dmitri-d and htuch Jun 8, 2021

htuch suggested changes Jun 9, 2021

View reviewed changes

repokitteh-read-only bot added the waiting label Jun 9, 2021

address reviewer comments

6c0d5e4

Signed-off-by: Mark D. Roth <[email protected]>

markdroth dismissed stale reviews from dmitri-d and krnowak via 6c0d5e4 June 10, 2021 15:48

repokitteh-read-only bot removed the waiting label Jun 10, 2021

htuch reviewed Jun 11, 2021

View reviewed changes

attempt to fix docs generation

5933bb6

Signed-off-by: Mark D. Roth <[email protected]>

htuch approved these changes Jun 11, 2021

View reviewed changes

htuch merged commit b164962 into envoyproxy:main Jun 11, 2021

markdroth deleted the xds_wildcard branch September 20, 2021 15:10

stevenzzzz reviewed Sep 22, 2021

View reviewed changes

markdroth mentioned this pull request Jan 5, 2023

Clarify that on-demand CDS does not support SotW xDS #24763

Merged

markdroth mentioned this pull request Nov 21, 2024

xRFC TP3: Error Propagation cncf/xds#107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xds: allow "*" to indicate a wildcard subscription #16861

xds: allow "*" to indicate a wildcard subscription #16861

markdroth commented Jun 7, 2021 •

edited

Loading

dmitri-d Jun 7, 2021

markdroth Jun 8, 2021

dmitri-d Jun 8, 2021

markdroth Jun 9, 2021

dmitri-d Jun 7, 2021

markdroth Jun 8, 2021

dmitri-d commented Jun 7, 2021

krnowak commented Jun 8, 2021

markdroth commented Jun 8, 2021

krnowak left a comment

dmitri-d commented Jun 8, 2021

htuch left a comment

htuch Jun 9, 2021

markdroth Jun 10, 2021

htuch Jun 9, 2021

htuch Jun 9, 2021

markdroth Jun 10, 2021

htuch left a comment

markdroth commented Jun 11, 2021

dmitri-d commented Jun 11, 2021

repokitteh-read-only bot commented Jun 11, 2021

htuch left a comment

stevenzzzz Sep 22, 2021

markdroth Sep 22, 2021

stevenzzzz Sep 22, 2021

krnowak Sep 22, 2021

markdroth Sep 22, 2021

stevenzzzz Sep 22, 2021

krnowak Sep 22, 2021

markdroth Sep 22, 2021

xds: allow "*" to indicate a wildcard subscription #16861

xds: allow "*" to indicate a wildcard subscription #16861

Conversation

markdroth commented Jun 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitri-d commented Jun 7, 2021

krnowak commented Jun 8, 2021

markdroth commented Jun 8, 2021

krnowak left a comment

Choose a reason for hiding this comment

dmitri-d commented Jun 8, 2021

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

markdroth commented Jun 11, 2021

dmitri-d commented Jun 11, 2021

repokitteh-read-only bot commented Jun 11, 2021

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markdroth commented Jun 7, 2021 •

edited

Loading