Delta XDS - improved

[slonka]

This is a PR based on work from @sschepens#152 and @mikegajdaHubSpot#4 .

At Allegro we've implemented it here: allegro/envoy-control#255 .

We've seen 50-70% improvements in CPUs usage in Envoys that do not have significant traffic (mostly do XDS) and ~35% in more RPS intensive apps.

[slonka]

@mikegajda - some of your commits are missing sign-off, do you mind if I squash them and add you as a co-author?

[mikegajda]

Sure that'd be great, thanks @slonka!

[Ferdudas97]

Tests on our environment are finished. We confirmed that, using delta improves cpu utilization ( dropped by ~50%).

[rulex123]

@Ferdudas97 Hello!

I have tried this patch out on Spotify's perimeter, which is Envoy based and uses a custom control plane that sends SoTW updates to our Envoy fleet. The only updates sent concern (atm) changes in clusters endpoints, and we have something like 350 upstream clusters configured in our Envoys.

I tried out this patch on one of our Envoys that serves production traffic (around 8.5K RPS), and didn't observe any noticeable changes in CPU usage, though I could verify that the Envoy was using incremental xDS . So I am curious about the setup you tried this on, since you noticed such a drastic improvement: could you share some details about the setup (e.g. number of clusters, type and frequency of changes etc.)?

[Ferdudas97]

Hi @rulex123, I tested it on our environment where we have ~1750 clusters. We are using envoy-control, which is our abstraction layer over java-control-plane. According to our metrics, it returns 200-600 snapshots per minute.

On our service with enabled DELTA_GRPC and wildcard dependencies (service is receiving SoTW), we can see CPU utilization drop by 50%.

Part of envoy's config

dynamic_resources:
lds_config:
ads: {}
resource_api_version: V3
initial_fetch_timeout: 20s
cds_config:
ads: {}
resource_api_version: V3
initial_fetch_timeout: 20s
ads_config:
api_type: DELTA_GRPC
transport_api_version: V3
grpc_services:
envoy_grpc:
cluster_name: envoy-control-xds

[rulex123]

I would like to start removing support for xDS v2 altogether, do you think it would be possible to coordinate that effort with adding support for xDS incremental? i.e. If we removed support for v2 and then updated this patch accordingly, the amount of code to test/review to ship xDS incremental would reduce significantly.

[Automaat]

I would like to start removing support for xDS v2 altogether, do you think it would be possible to coordinate that effort with adding support for xDS incremental? i.e. If we removed support for v2 and then updated this patch accordingly, the amount of code to test/review to ship xDS incremental would reduce significantly.

@rulex123 We don't want to merge these two topics, since these are two different topics. I have a local branch with almost removed v2 API and would like to push it as soon as we merge this pull request with delta XDS.

[rulex123]

I would like to start removing support for xDS v2 altogether, do you think it would be possible to coordinate that effort with adding support for xDS incremental? i.e. If we removed support for v2 and then updated this patch accordingly, the amount of code to test/review to ship xDS incremental would reduce significantly.

@rulex123 We don't want to merge these two topics, since these are two different topics. I have a local branch with almost removed v2 API and would like to push it as soon as we merge this pull request with delta XDS.

I agree we shouldn't merge the two in one patch: what I am suggesting is that we first ship removal of support for xDS v2, and after that change has been merged upstream, we rebase the delta-xds-non-breaking-slonka-hash-bytes branch on main so that the scope of the code to review/test in this patch is reduced.

I think this is worth it, especially if you have a branch where you started the v2 removal work. I can also help with that work, in case you don't have cycles to continue it. WDYT?

[Automaat]

I would like to start removing support for xDS v2 altogether, do you think it would be possible to coordinate that effort with adding support for xDS incremental? i.e. If we removed support for v2 and then updated this patch accordingly, the amount of code to test/review to ship xDS incremental would reduce significantly.

@rulex123 We don't want to merge these two topics, since these are two different topics. I have a local branch with almost removed v2 API and would like to push it as soon as we merge this pull request with delta XDS.

I agree we shouldn't merge the two in one patch: what I am suggesting is that we first ship removal of support for xDS v2, and after that change has been merged upstream, we rebase the delta-xds-non-breaking-slonka-hash-bytes branch on main so that the scope of the code to review/test in this patch is reduced.

I think this is worth it, especially if you have a branch where you started the v2 removal work. I can also help with that work, in case you don't have cycles to continue it. WDYT?

I will work on removing API v2 during this week, hopefully I will prepare first PR this week

[mikegajda]

👋 any updates here? I'd be happy to help get this merged as I'm hoping to deploy this change soon

[Ferdudas97]

👋 any updates here? I'd be happy to help get this merged as I'm hoping to deploy this change soon

Hey, the work is done on this branch. We are waiting for the review and merging of this PR -> there is a problem with publishing artifact for JCP.

[Ferdudas97]

@rulex123@mikegajda it is ready for your reviews

[mikegajda]

Taking a look at this in a moment @Ferdudas97! Thanks for getting it to this point

[mikegajda]

Should this be onV2StreamRequest to fulfill this the goal above "throws if it sees a v2 request", or were you thinking of throwing on a V3 non-delta request here and the comment above is in out of sync?

[Ferdudas97]

it was out of sync