N^2 RDS memory for SNI

[phylake]

Title: N^2 RDS memory for SNI

Description:
Every filter_chain_match in SNI has its own HTTP connection manager which gets its own copy of the full RDS config. For every filter chain match there's going to be a route entry (N^2).

I was looking at how to share the ConfigImpl between HTTP connection managers but what stumped me was the Server::Configuration::FactoryContext& factory_context that's passed down to ultimately be used by PerFilterConfig

1. What's the best way to address this short term? (i.e. a hack to reduce our memory footprint of 30GiB)
2. What's the real solution? (i.e. something I could contribute)
3. How does this intersect with @AndresGuedez's work?

/cc @lrouquette @bryanlatten

[phylake]

This is envoy_inuse_space for heap dumps taken until we hit the 30GiB steady state

[adrocknaphobia]

@atrifan ☝️

[mattklein123]

cc @htuch @fredlas @AndresGuedez @stevenzzzz

I think the real solution here is to allow the RDS watches to point to a single subscription and single shared route config. I think this is probably not that hard given the work that @fredlas just did and the work that @stevenzzzz is doing right now but I will let them weigh in.

[AndresGuedez]

I think the real solution here is to allow the RDS watches to point to a single subscription and single shared route config.

Agreed. This is one of the primary goals of the Config Provider framework that was introduced as part of the SRDS work.

Once the SRDS implementation is complete, we are planning to transition RDS to use the framework as well.

[mattklein123]

Sounds good I will assign this over to @stevenzzzz for now.

[lizan]

I think the real solution here is to allow the RDS watches to point to a single subscription and single shared route config.

@mattklein123 It does point to a single subscription but different route config due to #3960 to fix #3953. I believe this is already in the consideration of @AndresGuedez 's Config Provider framework.

#7870 could fix this from the other side by not having multiple HCM for different SNI.

[atrifan]

I think that what @AndresGuedez said is the only viable solution and there is no place for a “quick fix” or a “work-around”. Reloads, hot reloads, gracefull reloads and other in-place modifications need to happen without affecting other ongoing clients - this is the main responsability of a proxy so right now keeping a copy of the RDS for each filter_chain (which you can look at as a middleware of that request) is by design a safety mechanism so you don’t break other request handlings that are ongoing. Also keeping a copy makes sure that you update each filter chain when you are ready, that beeing later then another filter_chain or earlier. In general - other proxies that support gracefull updates (you can check out nginx as well) have a big problem with the memory footprint - in different phases though or diferrent situations.

[mattklein123]

@lizan yeah I think we should be able to get back to a shared route config for a shared subscription. I think this is also related to #7902 to which @stevenzzzz is working on.

@atrifan for identical route configs which point to the same named RDS subscription we can definitely make it shared. That is planned.

[htuch]

@stevenzzzz and I were discussed moving RDS to the config provider framework today. This might need to happen prior to SRDS landing due to some complexities around the lifetime management that are trickier than expected in #7451. @stevenzzzz is analyzing complexity of different approaches here last we spoke.

We definitely need to go sharing for memory use reasons, but at the same time, we need to be very careful how we structure this. There is a lot of lifetime and ownership complexity in RDS/SRDS that we need to make sure is maintainable and grokkable by developers.

[stevenzzzz]

yeah, migrate RDS to the config-provider-framework is on my radar.
Will do that after the SRDS impl.

[mattklein123]

I believe this is fixed by #9209