You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2018/03/04 19:24:51 UTC

Fix for ab defect (was: [VOTE] Release httpd-2.4.31)

On Sat, Mar 3, 2018 at 10:51 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Sat, Mar 3, 2018 at 6:40 PM, lists@rhsoft.net <li...@rhsoft.net> wrote:
>>
>> -1
>>
>> "ab" no longer can benchmark https urls, same build-spec and environment
>> (Fedora 26 and 27)
>
> Hmm, looks like 2.4 is missing http://svn.apache.org/r1580928 (second hunk).

Does it work for you with this patch (on top of 2.4.31):
  http://home.apache.org/~ylavic/patches/httpd-2.4.x-ab-nonblock_length.patch
?

Thanks for testing (if possible).

Regards,
Yann.

Re: [VOTE] Release httpd-2.4.31

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Mar 4, 2018 at 8:38 PM, lists@rhsoft.net <li...@rhsoft.net> wrote:
>
> Am 04.03.2018 um 20:33 schrieb Yann Ylavic:
>>
>> On Sun, Mar 4, 2018 at 8:27 PM, lists@rhsoft.net <li...@rhsoft.net> wrote:
>>>
>>>
>>> that patchfile is unuseable for rpmbuild
>>>
>>> + echo 'Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):'
>>> Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):
>>> + /usr/bin/patch --no-backup-if-mismatch -p1 --fuzz=0
>>> can't find file to patch at input line 5
>>
>>
>> It requires -p0 (instead of -p1).
>> Nevermind, does the attached one work?
>
>
> confirmed, thanks

Thank you for testing!

Re: [VOTE] Release httpd-2.4.31

Posted by "lists@rhsoft.net" <li...@rhsoft.net>.
Am 04.03.2018 um 20:33 schrieb Yann Ylavic:
> On Sun, Mar 4, 2018 at 8:27 PM, lists@rhsoft.net <li...@rhsoft.net> wrote:
>>
>> that patchfile is unuseable for rpmbuild
>>
>> + echo 'Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):'
>> Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):
>> + /usr/bin/patch --no-backup-if-mismatch -p1 --fuzz=0
>> can't find file to patch at input line 5
> 
> It requires -p0 (instead of -p1).
> Nevermind, does the attached one work?

confirmed, thanks

[root@testserver:~]$ rpm -q httpd
httpd-2.4.31-2.0.fc27.20180304.rh.sandybridge.x86_64

[root@testserver:~]$ ab -c 1 -n 10 https://www.google.com/
This is ApacheBench, Version 2.3 <$Revision: 1814468 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking www.google.com (be patient).....done


Server Software:
Server Hostname:        www.google.com
Server Port:            443
SSL/TLS Protocol:       TLSv1.2,ECDHE-ECDSA-CHACHA20-POLY1305,256,256
TLS Server Name:        www.google.com

Document Path:          /
Document Length:        269 bytes

Concurrency Level:      1
Time taken for tests:   1.202 seconds
Complete requests:      10
Failed requests:        0
Non-2xx responses:      10
Total transferred:      6700 bytes
HTML transferred:       2690 bytes
Requests per second:    8.32 [#/sec] (mean)
Time per request:       120.213 [ms] (mean)
Time per request:       120.213 [ms] (mean, across all concurrent requests)
Transfer rate:          5.44 [Kbytes/sec] received

Connection Times (ms)
               min  mean[+/-sd] median   max
Connect:       81   95  13.5     90     119
Processing:    19   25   7.1     21      40
Waiting:       19   24   7.2     21      40
Total:        103  120  17.0    114     159

Percentage of the requests served within a certain time (ms)
   50%    114
   66%    119
   75%    126
   80%    137
   90%    159
   95%    159
   98%    159
   99%    159
  100%    159 (longest request)

Re: [VOTE] Release httpd-2.4.31

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Mar 4, 2018 at 8:27 PM, lists@rhsoft.net <li...@rhsoft.net> wrote:
>
> that pacthfile is unuseable for rpmbuild
>
> + echo 'Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):'
> Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):
> + /usr/bin/patch --no-backup-if-mismatch -p1 --fuzz=0
> can't find file to patch at input line 5

It requires -p0 (instead of -p1).
Nevermind, does the attached one work?

Re: [VOTE] Release httpd-2.4.31

Posted by "lists@rhsoft.net" <li...@rhsoft.net>.

Am 04.03.2018 um 20:24 schrieb Yann Ylavic:
> On Sat, Mar 3, 2018 at 10:51 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> On Sat, Mar 3, 2018 at 6:40 PM, lists@rhsoft.net <li...@rhsoft.net> wrote:
>>>
>>> -1
>>>
>>> "ab" no longer can benchmark https urls, same build-spec and environment
>>> (Fedora 26 and 27)
>>
>> Hmm, looks like 2.4 is missing http://svn.apache.org/r1580928 (second hunk).
> 
> Does it work for you with this patch (on top of 2.4.31):
>    http://home.apache.org/~ylavic/patches/httpd-2.4.x-ab-nonblock_length.patch
> ?
> 
> Thanks for testing (if possible)

that pacthfile is unuseable for rpmbuild

+ echo 'Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):'
Patch #4 (httpd-2.4.x-ab-nonblock_length.patch):
+ /usr/bin/patch --no-backup-if-mismatch -p1 --fuzz=0
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: CHANGES
|===================================================================
|--- CHANGES    (revision 1825829)
|+++ CHANGES    (working copy)
--------------------------

Re: Fix for ab defect

Posted by "lists@rhsoft.net" <li...@rhsoft.net>.

Am 05.03.2018 um 15:48 schrieb Yann Ylavic:
> I meant that before the patch, "ab" already succeeded for (e.g.)
> https://localhost/ or https://192.168.x.x/ that is if the connect is
> quick enough to not trigger the bug (though it's not necessarily the
> case in local networks either).
> This is probably why we didn't notice it on manual testing, "ab"-ing
> external/wan/google servers is not that usual...

FWIW - i noticed the bug on every single https request on the local 
machine, google.com was only for a reproducer

Concurrency Level:      1
Requests per second:    311.85 [#/sec] (mean)




Re: Fix for ab defect (was: [VOTE] Release httpd-2.4.31)

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Mar 5, 2018 at 3:16 AM, Daniel Ruggeri <DR...@primary.net> wrote:
>
>> -----Original Message-----
>> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>> Sent: Sunday, March 04, 2018 5:09 PM
[]
>> In this case though, this is not exactly "100% failure" in any
>> circonstances, for instance on localhost (or fast enough local
>> network) it won't fail since the errorneous path is not taken when
>> non-blocking connect succeeds.
>> It might not be easy/wise to launch/automate "ab" on an external
>> server in a test suite...
>
> I just added r1825841 to stub out some very basic ab tests (and
> r1825842 now that I noticed a shortcoming). I'm not sure about the
> statement above. Maybe I misunderstand, but with my tests
> before/after the patch, the new test can detect this particular
> failure and should at least also protect us from trying to ship an ab
> build that returns non-zero and has anything in STDERR under normal
> circumstances. Review much appreciated, of course.

I meant that before the patch, "ab" already succeeded for (e.g.)
https://localhost/ or https://192.168.x.x/ that is if the connect is
quick enough to not trigger the bug (though it's not necessarily the
case in local networks either).
This is probably why we didn't notice it on manual testing, "ab"-ing
external/wan/google servers is not that usual...
So I wonder which server is used by r1825841, and if everyone can run
the test from home with being banned by gg.com or a.o ;)

Thanks for the test anyway (no idea about perl/pipe things :/ ).

Regards,
Yann.

RE: Fix for ab defect (was: [VOTE] Release httpd-2.4.31)

Posted by Daniel Ruggeri <DR...@primary.net>.
> -----Original Message-----
> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Sent: Sunday, March 04, 2018 5:09 PM
> To: httpd-dev <de...@httpd.apache.org>
> Subject: Re: Fix for ab defect (was: [VOTE] Release httpd-2.4.31)
> 
> On Sun, Mar 4, 2018 at 11:48 PM, Daniel Ruggeri <DR...@primary.net>
> wrote:
> >
> > I'd like to ask a followup question... how do we catch this in the
> > test suite? With this (100% failure), ab still returns a 0 exit code.
> > It *does* at least give the error message to STDERR. Perhaps we
> > should add to the test suite that `ab -q` completed against the http
> > and https vshosts with no lines printed to STDERR and has a 0 exit
> > code?
> 
> The best way is probably to capture stderr...
> In this case though, this is not exactly "100% failure" in any
> circonstances, for instance on localhost (or fast enough local
> network) it won't fail since the errorneous path is not taken when
> non-blocking connect succeeds.
> It might not be easy/wise to launch/automate "ab" on an external
> server in a test suite...

I just added r1825841 to stub out some very basic ab tests (and r1825842 now that I noticed a shortcoming). I'm not sure about the statement above. Maybe I misunderstand, but with my tests before/after the patch, the new test can detect this particular failure and should at least also protect us from trying to ship an ab build that returns non-zero and has anything in STDERR under normal circumstances. Review much appreciated, of course.

What I greatly dislike about the above commit is that (at least on my tests), the STDERR and STDOUT from the child process appears to be folded into STDOUT. Thus, I added a failsafe check that STDOUT doesn't contain what looks to be an SSL error. This may be a side effect of the test suite because when running the same command in a standard shell the SSL complaint is on STDERR.

I'm wondering if anyone can explain that behavior since IPC::Open3 has always segregated these streams?

> 
> Regards,
> Yann.

-- 
Daniel Ruggeri


Re: Fix for ab defect (was: [VOTE] Release httpd-2.4.31)

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Mar 4, 2018 at 11:48 PM, Daniel Ruggeri <DR...@primary.net> wrote:
>
> I'd like to ask a followup question... how do we catch this in the
> test suite? With this (100% failure), ab still returns a 0 exit code.
> It *does* at least give the error message to STDERR. Perhaps we
> should add to the test suite that `ab -q` completed against the http
> and https vshosts with no lines printed to STDERR and has a 0 exit
> code?

The best way is probably to capture stderr...
In this case though, this is not exactly "100% failure" in any
circonstances, for instance on localhost (or fast enough local
network) it won't fail since the errorneous path is not taken when
non-blocking connect succeeds.
It might not be easy/wise to launch/automate "ab" on an external
server in a test suite...

Regards,
Yann.

RE: Fix for ab defect (was: [VOTE] Release httpd-2.4.31)

Posted by Daniel Ruggeri <DR...@primary.net>.
I've tested the patch against 2.4.31 as provided in STATUS and confirmed it fixes the issue. Thanks for the very fast turnaround.

I'd like to ask a followup question... how do we catch this in the test suite? With this (100% failure), ab still returns a 0 exit code. It *does* at least give the error message to STDERR. Perhaps we should add to the test suite that `ab -q` completed against the http and https vshosts with no lines printed to STDERR and has a 0 exit code?

-- 
Daniel Ruggeri

> -----Original Message-----
> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Sent: Sunday, March 04, 2018 1:25 PM
> To: lists@rhsoft.net
> Cc: httpd-dev <de...@httpd.apache.org>
> Subject: Fix for ab defect (was: [VOTE] Release httpd-2.4.31)
> 
> On Sat, Mar 3, 2018 at 10:51 PM, Yann Ylavic <yl...@gmail.com> wrote:
> > On Sat, Mar 3, 2018 at 6:40 PM, lists@rhsoft.net <li...@rhsoft.net> wrote:
> >>
> >> -1
> >>
> >> "ab" no longer can benchmark https urls, same build-spec and
> environment
> >> (Fedora 26 and 27)
> >
> > Hmm, looks like 2.4 is missing http://svn.apache.org/r1580928 (second
> hunk).
> 
> Does it work for you with this patch (on top of 2.4.31):
>   http://home.apache.org/~ylavic/patches/httpd-2.4.x-ab-
> nonblock_length.patch
> ?
> 
> Thanks for testing (if possible).
> 
> Regards,
> Yann.