ads: aggregated discovery service.#124
Conversation
See README.md for motivation and usage.
| service AggregatedDiscoveryService { | ||
| // This is a gRPC-only API. | ||
| rpc StreamAggregatedResources(stream DiscoveryRequest) | ||
| returns (stream DiscoveryResponse) { |
There was a problem hiding this comment.
Even though we have dropped the canary conversion for now, do we still want to make the API a stream of "atomic" lists of xDS updates? I see no downside since from an implementation perspective initially it could just still go through them in order, but in the future it would allow us to to "atomic" things more easily without an API change?
There was a problem hiding this comment.
I considered this and opted for simplicity in this PR, but I'm open to adopting the atomic list concept if we can provide a concrete use case. By moving to lists of updates in each response we also make the requests more complicated, as they need to express multiple version numbers (for each singleton xDS API that gets updated or fails its update). I think it's worth doing if we have a strong motivator here, but all things being equal I'm tending towards simplicity over future proofing.
There's a nice property of the existing API that a server that can handle ADS requests can also handle arbitrary singleton xDS requests as well, if we add more structure to the ADS request then this goes away, but maybe it's not that important.
| distinct streams, one for *X* and one for *Y*. ADS allows these to be | ||
| coalesced to a single stream to a single management server, avoiding the need | ||
| for distributed synchronization to correctly sequence the update. With ADS, | ||
| the management server would deliver the CDS, EDS and then RDS updates on a |
There was a problem hiding this comment.
What about LDS updates? Should the sequence be CDS, EDS,RDS and then LDS?
There was a problem hiding this comment.
LDS must come before RDS so the need for RDS is setup. LDS update will not become active until RDS is initialized (which could happen very quickly if it's the next message processed).
There was a problem hiding this comment.
In this example, the idea was that LDS wasn't changing, it was purely routes and clusters. Above I mention the sequence as [CDS, EDS, LDS, RDS]. This is because LDS will warm on RDS and we allow Envoy to be aware of all the expected RDS resources via LDS ahead of the RDS delivery. If there' some reason it should be RDS and then LDS, then we should change things.
There was a problem hiding this comment.
Yeah good point about version numbers. OK let's just keep it simple. We can always add another API later if needed.
See README.md for motivation and usage.
Fixes #102.