On Wed, Feb 12, 2025 at 12:38 PM Yann Ylavic <[email protected]> wrote:
>
> However it seems that [4] will cause worker->s->is_address_reusable =
> 0 in [3] (i.e. disable DNS reuse altogether) which is not really
> expected, and would explain why the pool is cleared in
> connection_cleanup().
>
> [3]
> https://github.com/apache/httpd/blob/2.4.63/modules/proxy/proxy_util.c#L1989-L2034
> [4]
> https://github.com/apache/httpd/blob/2.4.63/modules/proxy/proxy_util.c#L2140-L2159
I think we need something like the attached patch to not let the
->disablereuse=1 in [4] (because of a $-substitution including outside
hostname[:port]) force ->is_address_reusable=0 in [3] implicitly.
The rationale is that before 2.4.59 ->is_address_reusable and
->disablereuse were kind of the same thing (the former, not
configurable, was set from the latter), but since 2.4.59
->is_address_reusable is about the reusability of backend's DNS
address (and lifetime, both deduced from the address_ttl= parameter)
while ->disablereuse is about the reusability of the
connections/sockets only.
Except we probably still want to disable address reuse when
disablereuse=on explicitly (for compatibility), but not for the
implicit case [4], which is what this patch does.
Thoughts?
Jean-Frederic, I don't think it addresses your issue because you
probably have enablereuse=on already if your ProxyPassMatch used to
reuse connections before 2.4.59? Connection reuse for ProxyPassMatch
is possible since 2.4.47, but has always depended on enablereuse=on.
If you don't use ProxyPassMatch with a $-substitution I don't see why
something changed in 2.4.59 though..
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 1923126)
+++ modules/proxy/proxy_util.c (working copy)
@@ -2262,12 +2262,18 @@ PROXY_DECLARE(apr_status_t) ap_proxy_initialize_wo
if (!worker->s->retry_set) {
worker->s->retry = apr_time_from_sec(PROXY_WORKER_DEFAULT_RETRY);
}
- /* Consistently set address and connection reusabilty: when reuse
- * is disabled by configuration, or when the address is known already
- * to not be reusable for this worker (in any case, thus ignore/force
- * DisableReuse).
+ /* worker->s->disablereuse is about disabling backend connection/socket
+ * reuse for successive requests, while worker->s->is_address_reusable
+ * is about DNS address reuse (an worker->s->address_ttl of zero also
+ * means not reusable). When the DNS address is not reusable neither
+ * are the connections, so make this consistent here.
+ * However setting disablereuse=on (or enablereuse=off) has always
+ * disabled DNS address reuse too, both were kind of the same thing
+ * before 2.4.59, so we keep this behaviour unless an address_ttl= is
+ * configured explicitely.
*/
if (!worker->s->address_ttl || (!worker->s->address_ttl_set
+ && worker->s->disablereuse_set
&& worker->s->disablereuse)) {
worker->s->is_address_reusable = 0;
}
