ads: Envoy sending DeltaDiscoveryRequest for each EDS resource

[sschepens]

When using CDS+EDS over ADS, when envoy receives a CDS response with new clusters it is sending a separate DeltaDiscoveryRequest for each resource instead of a single DeltaDiscoveryRequest with multiple resource_names_subscribe .

I'm using envoy 1.14.1

[sschepens]

here are some logs from our controlplane:

[log_time:17:52:59.610] - [0] Got delta stream request type_url: "type.googleapis.com/envoy.config.cluster.v3.Cluster"

[log_time:17:52:59.644] - [0] Got delta stream response: system_version_info: "1589909822292_nXYGHA=="
	resources {
	  version: "1589905705182_cT_d7A=="
	  resource {
	    type_url: "type.googleapis.com/envoy.config.cluster.v3.Cluster"
	    value: "VALUE"
	  }
	  name: "cluster1"
	}
	resources {
	  version: "1589909822292_7Enb8A=="
	  resource {
	    type_url: "type.googleapis.com/envoy.config.cluster.v3.Cluster"
	    value: "VALUE"
	  }
	  name: "cluster2"
	}
	type_url: "type.googleapis.com/envoy.config.cluster.v3.Cluster"
	nonce: "2"

[log_time:17:52:59.653] - [0] Got delta stream request type_url: "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment"
	resource_names_subscribe: "cluster1"
	 
[log_time:17:52:59.654] - [0] Got delta stream request type_url: "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment"
	resource_names_subscribe: "cluster2"

[log_time:17:52:59.654] - [0] Got delta stream request type_url: "type.googleapis.com/envoy.config.cluster.v3.Cluster"
	response_nonce: "2"

[mattklein123]

I believe this is currently by design based on how EDS works, because it is split per cluster and there really is no "delta EDS". See #10373.

[sschepens]

Makes sense that it could be by current design but @htuch suggested this could be a bug.

I don't get though why LEDS would solve this and if there is no current way of fixing this.

[htuch]

It's true that there is no real delta EDS at the sub-cluster level, but I don't think there should be a separate EDS DeltaDiscoveryRequest for every cluster in a CDS update. This is the same issue in SotW that we fixed with the pause()/resume() feature.

[sschepens]

@htuch i found this line which presumably is only pausing EDS on V2 api version.

We're using envoy with resource_api_version: V3 and transport_api_version: V3, i'm guessing this could be causing the issue.

Is it OK for it to be fixed to V2 resources only?

[htuch]

Yeah, I think @Shikugawa was working on something here but might not have landed (or maybe I'm misremembering). In any case, we should pause/resume both the v2 and v3 CLAs.

[sschepens]

@htuch i created #11300 could you see if that does the trick? I didn't seem to find any tests regarding pausing/resuming

[mattklein123]

Makes sense that it could be by current design but @htuch suggested this could be a bug.

Sorry I can't keep all of this straight, especially with ADS. So this is purely a perf enhancement, right? Meaning, it shouldn't actually matter if all of them come in different requests or for ADS is that required?

[htuch]

@sschepens looks like you'e on the right track, commented.

@mattklein123 yep, a performance enhancement as you describe.

[htuch]

Tagging for #10943