Re: svn commit: r1750474 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_ajp.c mod_proxy_fcgi.c mod_proxy_http.c proxy_util.c

[Yann Ylavic]

On Tue, Jun 28, 2016 at 4:49 PM, Ruediger Pluem  wrote:
>
>
> On 06/28/2016 04:26 PM, Ruediger Pluem wrote:
>>
>>
>> On 06/28/2016 01:19 PM, yla...@apache.org wrote:
>>> Author: ylavic
>>> Date: Tue Jun 28 11:19:36 2016
>>> New Revision: 1750474
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1750474=rev
>>> Log:
>>> mod_proxy: follow up to r1750392.
>>> Avoid double checking the connection in ap_proxy_connect_backend() when
>>> ap_proxy_check_backend() says it is up and good to go.
>>>
>>> This can be done by moving the PROXY_WORKER_IS_USABLE() check in
>>> ap_proxy_check_backend(), since it is called by ap_proxy_connect_backend(),
>>> and not calling the latter if the former succeeded (for the modules using 
>>> it).
>>
>> IMHO this is a bad idea, because ap_proxy_connect_backend does more then 
>> just checking the socket, even if it is connected.
>>
>> See lines 2915 till 2955.
>
> Typed faster than reading. I see this is already addressed.

Yes, it didn't pass the tests framework (useful!).
Thanks for the review anyway.

Regards,
Yann.

Re: svn commit: r1750474 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_ajp.c mod_proxy_fcgi.c mod_proxy_http.c proxy_util.c

[Ruediger Pluem]

On 06/28/2016 04:26 PM, Ruediger Pluem wrote:
> 
> 
> On 06/28/2016 01:19 PM, yla...@apache.org wrote:
>> Author: ylavic
>> Date: Tue Jun 28 11:19:36 2016
>> New Revision: 1750474
>>
>> URL: http://svn.apache.org/viewvc?rev=1750474=rev
>> Log:
>> mod_proxy: follow up to r1750392.
>> Avoid double checking the connection in ap_proxy_connect_backend() when
>> ap_proxy_check_backend() says it is up and good to go.
>>
>> This can be done by moving the PROXY_WORKER_IS_USABLE() check in
>> ap_proxy_check_backend(), since it is called by ap_proxy_connect_backend(),
>> and not calling the latter if the former succeeded (for the modules using 
>> it).
> 
> IMHO this is a bad idea, because ap_proxy_connect_backend does more then just 
> checking the socket, even if it is connected.
> 
> See lines 2915 till 2955.

Typed faster than reading. I see this is already addressed.

Regards

Rüdiger

Re: svn commit: r1750474 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_ajp.c mod_proxy_fcgi.c mod_proxy_http.c proxy_util.c

[Ruediger Pluem]

On 06/28/2016 01:19 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Tue Jun 28 11:19:36 2016
> New Revision: 1750474
> 
> URL: http://svn.apache.org/viewvc?rev=1750474=rev
> Log:
> mod_proxy: follow up to r1750392.
> Avoid double checking the connection in ap_proxy_connect_backend() when
> ap_proxy_check_backend() says it is up and good to go.
> 
> This can be done by moving the PROXY_WORKER_IS_USABLE() check in
> ap_proxy_check_backend(), since it is called by ap_proxy_connect_backend(),
> and not calling the latter if the former succeeded (for the modules using it).

IMHO this is a bad idea, because ap_proxy_connect_backend does more then just 
checking the socket, even if it is connected.

See lines 2915 till 2955.

Regards

Rüdiger