On Mon, May 19, 2025 at 4:05 PM Ruediger Pluem <rpl...@apache.org> wrote:
>
>
>
> On 5/19/25 1:53 PM, jean-frederic clere wrote:
> > Hi,
> >
> > While investigating a customer case I note a regression with forward proxy.
> > While testing I have:
> > 2.4.57 finished in 3.05s, 3278.62 req/s, 743.70KB/s
> > 2.4.x finished in 15.38s, 650.17 req/s, 147.88KB/s
> >
> > The regression goes way with:
> > +++
> > jfclere@fedora:~/httpd-2.4.x$ svn diff .
> > Index: modules/proxy/proxy_util.c
> > ===================================================================
> > --- modules/proxy/proxy_util.c (revision 1925472)
> > +++ modules/proxy/proxy_util.c (working copy)
> > @@ -1660,7 +1660,6 @@
> > conn = connection_make(p, worker);
> > }
> > else if (conn->close
> > - || conn->forward
> > || (conn->connection
> > && conn->connection->keepalive == AP_CONN_CLOSE)
> > || worker->s->disablereuse) {
> > +++
> > But that doesn't look quite right. any hints?
>
> Agreed that is does not look right. What configuration do you use for the
> test?
> What is the test scenario?
Hm, r1912459 added this test to disable reuse for CONNECT connections
to ProxyRemote, because I thought that CONNECT requests were not
reusable. However this kinds of defeats the code in
ap_proxy_determine_connection() allowing to reuse a ProxyRemote
connection [1] if its forward_info did not change (same target
host/port and same auth), and I think that's what your use case was
reaching before r1912459.
It's probably safe for successive requests to the same host/port/auth
to be pipelined in the same CONNECT payload, saving new connections
and TLS handshakes, so possibly Jean-Frederic's patch is correct
(though not complete, we need to close the connection if the
forward_info does not match in [1])?
[1] https://github.com/apache/httpd/blob/2.4.63/modules/proxy/proxy_util.c#L3372
Regards;
Yann.
