httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: ap_proxy_determine_connection() made simpler?
Date Fri, 24 Jan 2014 17:56:04 GMT
On Fri, Jan 24, 2014 at 2:07 PM, Jim Jagielski <jim@jagunet.com> wrote:

> Hmmm from what I can see, if we have a hostname and the address
> is re-usable, we do stuff we shouldn't.
>

The patch does nothing in the case you mention (like the original code),
but I guess you meant "we have a hostnane and the address is *not*
reusable". In which case the patch does nothing either, whereas the
original code overwrittes the hostname, close the socket and then issues a
new DNS lookup.

That's indeed more than applying the De Morgan's law, but maybe I can
explain why this is still a streamlining of the whole connections/adresses
reuse logic.

First let me stress that currently, !is_address_reusable is strictly
equivalent to disablereuse, they imply the same paths everywhere.
Whenever one or the other is in action (!is_address_reusable ||
disablereuse) the connection *and* address are never reused, and when both
are not in action (is_address_reusable && !disablereuse) the connection
*and* address are always reused.
Disabling socket reuse only will still issue a DNS lookup for each request.
Disabling address reuse only will still close the socket when done.

Hence there is no distinction between connection and address reuse, the 2
flags could be merged into a single one, although it would make sense to
keep both and make them do what they mean :
1. disablereuse: close the socket on release; ignore the flag on acquire
(just compare conn->sock with NULL)
2. !is_address_reusable: resolve address on acquire (unconditionnaly), and
if the socket is reused compare with its address (sockaddr_equal) and close
it on mismatch; ignore the flag on release (just compare
conn->hostname/addr with NULL)
3. the current behaviour is implied by 1. and 2. when is_address_reusable
== !disablereuse

Any thoughts?

As you can see, my proposed patch is influenced by my current work on
addresses' lifetime (DNS lookups), for which I feel quite unconfortable
with the current/mixed "reuse" implementation.

Back to the patch then...
It indeed assumes that when the hostname is not NULL, the connection
(address/socket) is reusable (otherwise it would have been cleared/zeroed
on release).
Hence it will never issue a DNS lookup nor socket close in that case (not
cleared => reusable).
Thus, the single/per-request DNS lookup is done only when hostname is NULL,
and solely depends on worker's reuse parameters.
I don't think it breaks anything, unless one (not httpd AFAICT) forces
conn->hostname to something which is not to be taken into account...



> r1560979 is my streamlining...
>

Yes, nice De Morgan's law.

Do you plan to address my comment regarding conn->hostname's leak on
http://mail-archives.apache.org/mod_mbox/httpd-dev/201401.mbox/%3CCAKQ1sVPT_-+cndcGH95Pt4AdF30kN6+wjAL3w15QORewapR64g@mail.gmail.com%3E?



>
> Thx for the idea
>

Yw. I'll come back with the patch described above (distinct
socket/address reuse) if it sounds applicable.
Comments on this are welcome...

Regards,
Yann.

Mime
View raw message