httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: [Patch] Keep Alive not workwing with mod_proxy (PR38602)
Date Mon, 13 Feb 2006 16:22:15 GMT
This looks like a big change, and my only concern is
that the behavior changes, although it appears that
we don't know why the current behavior is the
way it is...

Anyway:

On Feb 12, 2006, at 3:53 PM, Ruediger Pluem wrote:
>
> The real problem is that we actually *close* our connection to the  
> backend
> after each request (see line 1512 of mod_proxy_http.c) and if it  
> would have
> survived we would *close* it on the reusal of this connection:
>

Again, this appears to be specifically done that way
for a reason, but I have no idea what it would
have been :)

> @@ -1504,12 +1513,6 @@
>
>                      /* found the last brigade? */
>                      if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
> -                        /* if this is the last brigade, cleanup the
> -                         * backend connection first to prevent the
> -                         * backend server from hanging around waiting
> -                         * for a slow client to eat these bytes
> -                         */
> -                        backend->close = 1;
>                          /* signal that we must leave */
>                          finish = TRUE;
>                      }

This, I think provides a clue: I'm guessing we are trying
to optimize the client-Apache link, at the expense of
opening/closing sockets to the backend, or wasting
those sockets. If we had a nice connection pool, then
it would be different...

> @@ -1667,26 +1659,15 @@
>               "proxy: HTTP: serving URL %s", url);
>
>
> -    /* only use stored info for top-level pages. Sub requests  
> don't share
> -     * in keepalives
> -     */
> -    if (!r->main) {
> -        backend = (proxy_conn_rec *) ap_get_module_config(c- 
> >conn_config,
> -                                                       
> &proxy_http_module);
> -    }
>      /* create space for state information */
>      if (!backend) {
>          if ((status = ap_proxy_acquire_connection(proxy_function,  
> &backend,
>                                                    worker, r- 
> >server)) != OK)
>              goto cleanup;
>
> -        if (!r->main) {
> -            ap_set_module_config(c->conn_config,  
> &proxy_http_module, backend);
> -        }
>      }
>
>

Not sure why we would bother still having that !backend
check, since we know it's NULL. We set it to NULL :)
And this also seems to allude to the fact that the
present framework is to support pooled connections.
Not sure how the above would conflict with subrequests

Does the patched version pass the test framework?

Mime
View raw message