Make proxy.config.http.per_server.min_keep_alive_connections overridable

[sudheerv]

Allows to customize min keep alives for services depending on their qps
For low qps services, when Host based server session match is configured
a floor on min keep alive, may cause TrafficServer to pick stale connections
even when a server is made inactive.

Note also that I've changed the config name from proxy.config.http.per_server.min_keep_alive to proxy.config.http.per_server.connection.min to match the other related settings.

https://docs.trafficserver.apache.org/en/latest/admin-guide/files/records.config.en.html#proxy-config-http-per-server-min-keep-alive-connections

[SolidWallOfCode]

Why does min need to be set here, but not max? Because max is not overridable?

[sudheerv]

Not really - max is just as overridable as min would be after this PR. The reason for not updating max into Group is that, the configured max value is only needed when making a new Origin connection (to check if the current count is past the max), and we always have a valid TXN when making a new origin connection.

However, that's not true for min - min value is needed when closing an existing idle (keep alive) connection, which generally happens because of a VC level event (an error, or a keep alive timeout etc) and it isn't a guaranteed to have a valid TXN object (and therefore it's config) to compare against :(

If it makes sense to keep it consistent and remove confusion, I'm okay to add max also to the Group, but, it'd be redundant.

[SolidWallOfCode]

I'm not very happy with this implementation, primarily because of the bad mismatch between transaction overrides and outbound session level data. For example, the actual min value is set randomly based on which transaction happens to be the first to connect to the upstream. If all transactions override with the same value, this doesn't matter, but what if due to some misconfiguration (typographic or miscommunication) different values are configured?

Unfortunately addressing that problem requires changes at a significantly deeper level - transaction based overrides can never overcome it.

[sudheerv]

I'm not very happy with this implementation, primarily because of the bad mismatch between transaction overrides and outbound session level data. For example, the actual min value is set randomly based on which transaction happens to be the first to connect to the upstream. If all transactions override with the same value, this doesn't matter, but what if due to some misconfiguration (typographic or miscommunication)?

Unfortunately addressing that problem requires changes at a significantly deeper level - transaction based overrides can never overcome it.

Yeah, agree. I think the only type of override that's possible, with these kind of settings is an Origin level override. It does not make sense to override these at a TXN level, for the reasons you described. The name of these settings does indicate that it's per_server, but, I agree it's not something that's super obvious and needs a lot of context. Perhaps, it's outside the scope of this PR, but, may be, we should consider adding a new Origin based override API (and update conf_remap to use that) and make TSHttpTxnConfigXXXSet to fail an attempt to override settings that don't lend themselves to naturally align with overriding at TXN level

[SolidWallOfCode]

@a-canary is supposed to be working on some infrastructure to deal with this, it was one of the points of the Inheritable work. Definitely need to work something out at the summit.

[sudheerv]

@a-canary is supposed to be working on some infrastructure to deal with this, it was one of the points of the Inheritable work. Definitely need to work something out at the summit.

👍