do not build catch all virtual host every time

[ramaraochavali]

CatchAllVirtualHost only changes when sidecar for a proxy is changed. We do not have to build it for every route every time.

• Configuration Infrastructure
• Docs
• Installation
• Networking
• Performance and Scalability
• Policies and Telemetry
• Security
• Test and Release
• User Experience
• Developer Infrastructure

[howardjohn]

/test benchmark_istio

[howardjohn]

Benchmark test shows about a 3% increase maybe. Is that what you expected or do you have setups where its even more impactful?

I am 100% fine with this but am slightly worried about this making #27826 a bit worse. We need to fix that regardless for sure though...

The other question I had was - where do we stop with this? A natural next step would be to cache passthrough cluster, etc. I guess since those are only 1 instance its not a big deal vs this being 1 per route config.
It would be slightly preferred if we just generate the 1 catch all in the RDS gen (instead of in push context), but maybe that would require way to much refactoring to make it viable? That way all the route generation happens in the "right" spot, which is slightly useful for code organization and runtime performance. For example, we are now pushing route generation out of the route generation code, so telemetry is slightly skewed for xds gen timings, etc. Not a big deal though.

[ramaraochavali]

Benchmark test shows about a 3% increase maybe. Is that what you expected or do you have setups where its even more impactful?

No. But I was hoping when we scale to more routes and proxies, it might help.

I am 100% fine with this but am slightly worried about this making #27826 a bit worse. We need to fix that regardless for sure though...

Yes Agree.

The other question I had was - where do we stop with this? A natural next step would be to cache passthrough cluster, etc. I guess since those are only 1 instance its not a big deal vs this being 1 per route config.
It would be slightly preferred if we just generate the 1 catch all in the RDS gen (instead of in push context), but maybe that would require way to much refactoring to make it viable? That way all the route generation happens in the "right" spot, which is slightly useful for code organization and runtime performance. For example, we are now pushing route generation out of the route generation code, so telemetry is slightly skewed for xds gen timings, etc. Not a big deal thoug

I was going back and forth on this specific thing. That would mean we have to change a lot.

[ramaraochavali]

@howardjohn WDYT about this? Should we move forward or should we try to refactor it to rds? I can try that if you prefer that approach - but not sure how much refactoring it will be.

[YonkaFang]

Benchmark test shows about a 3% increase maybe. Is that what you expected or do you have setups where its even more impactful?

No. But I was hoping when we scale to more routes and proxies, it might help.

I am 100% fine with this but am slightly worried about this making #27826 a bit worse. We need to fix that regardless for sure though...

Yes Agree.

The other question I had was - where do we stop with this? A natural next step would be to cache passthrough cluster, etc. I guess since those are only 1 instance its not a big deal vs this being 1 per route config.
It would be slightly preferred if we just generate the 1 catch all in the RDS gen (instead of in push context), but maybe that would require way to much refactoring to make it viable? That way all the route generation happens in the "right" spot, which is slightly useful for code organization and runtime performance. For example, we are now pushing route generation out of the route generation code, so telemetry is slightly skewed for xds gen timings, etc. Not a big deal thoug

I was going back and forth on this specific thing. That would mean we have to change a lot.

@howardjohn It looks like what u worried has happened according to #36357