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 2016/04/13 22:04:55 UTC

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

On Thu, Feb 25, 2016 at 11:27 AM,  <ic...@apache.org> wrote:
> Author: icing
> Date: Thu Feb 25 10:27:27 2016
> New Revision: 1732275
>
> URL: http://svn.apache.org/viewvc?rev=1732275&view=rev
> Log:
> merging pre_close_connection hook, prep_lingering_close and ap_update_child() additions from trunk
>
> Modified:
[]
>     httpd/httpd/branches/2.4.x/server/scoreboard.c
[]
>
> Modified: httpd/httpd/branches/2.4.x/server/scoreboard.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/scoreboard.c?rev=1732275&r1=1732274&r2=1732275&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/server/scoreboard.c (original)
> +++ httpd/httpd/branches/2.4.x/server/scoreboard.c Thu Feb 25 10:27:27 2016
> @@ -32,6 +32,7 @@
>  #include "http_main.h"
>  #include "http_core.h"
>  #include "http_config.h"
> +#include "http_protocol.h"
>  #include "ap_mpm.h"
>
>  #include "scoreboard.h"
> @@ -457,7 +458,9 @@ static int update_child_status_internal(
>                                          int thread_num,
>                                          int status,
>                                          conn_rec *c,
> -                                        request_rec *r)
> +                                        server_rec *s,
> +                                        request_rec *r,
> +                                        const char *descr)
>  {
>      int old_status;
>      worker_score *ws;
[]
> @@ -489,28 +495,44 @@ static int update_child_status_internal(
>              }
>              ws->conn_count = 0;
>              ws->conn_bytes = 0;
> +            ws->last_used = apr_time_now();
>          }
> -        if (r) {
> -            const char *client = ap_get_remote_host(c, r->per_dir_config,
> -                                 REMOTE_NOLOOKUP, NULL);
> -            if (!client || !strcmp(client, c->client_ip)) {
> -                apr_cpystrn(ws->client, r->useragent_ip, sizeof(ws->client));
> +        if (status == SERVER_READY) {
> +            ws->client[0]='\0';
> +            ws->vhost[0]='\0';
> +            ws->request[0]='\0';
> +            ws->protocol[0]='\0';
> +        }

This seems to have changed mod_status' output.
Prior to this change the previous values were displayed until the
worker was reused for another connection/request, now it is reset as
soon as the worker is idle (i.e. each connection/request end).

I prefer the new behaviour, but there are some concerns on users@...

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by olli hauer <oh...@gmx.de>.
On 2016-04-14 21:47, Eric Covener wrote:
> On Thu, Apr 14, 2016 at 3:40 PM, olli hauer <oh...@gmx.de> wrote:
>> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...
> 
> What was your MaxSpareThreads during the test?
> 

The system is running with the default settings:

<IfModule mpm_event_module>
    StartServers             3
    MinSpareThreads         75
    MaxSpareThreads        250
    ThreadsPerChild         25
    MaxRequestWorkers      400
    MaxConnectionsPerChild   0
</IfModule>


Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Eric Covener <co...@gmail.com>.
On Thu, Apr 14, 2016 at 3:40 PM, olli hauer <oh...@gmx.de> wrote:
> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...

What was your MaxSpareThreads during the test?

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by olli hauer <oh...@gmx.de>.
On 2016-04-14 22:14, Yann Ylavic wrote:
> On Thu, Apr 14, 2016 at 10:05 PM, olli hauer <oh...@gmx.de> wrote:
>> On 2016-04-14 21:48, Yann Ylavic wrote:
>>> On Thu, Apr 14, 2016 at 9:40 PM, olli hauer <oh...@gmx.de> wrote:
>>>> I've done a quick test with
>>>>  $ ab -n 10000 -c 100 $host/$url
>>>>
>>>> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...
>>>
>>> This looks normal to me, did the bahaviour changed w.r.t. 2.4.19?
>>>
>>> When the stress is running, the number of idlers (workers that do
>>> nothing) fluctuates, according to the number of connections/requests
>>> and children created to sustain the load.
>>>
>>> When the load stops, the number of idlers increases, and won't
>>> decrease unless MaxSpareThreads is reached (i.e. children are stopped,
>>> each releasing ThreadsPerChild idlers).
>>>
>>
>> Ah, OK this will explain the numbers.
>> Unluckily I haven't looked to scoreboard with 2.4.19, but give me some minutes ...
> 
> Could you also confirm with 2.4.20 + my patch that the Client, VHost
> and Request colomns are always filled with latest values (not empty)
> for idle workers (used at least once)?
> 
> This requires "ExtendedStatus on" (or default) for the corresponding
> table to be displayed by mod_status.
> 
> Thanks for testing!
> 

Yes, I can confirm this, see the log in the answer to Rainer.

-- 
olli

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Apr 14, 2016 at 10:05 PM, olli hauer <oh...@gmx.de> wrote:
> On 2016-04-14 21:48, Yann Ylavic wrote:
>> On Thu, Apr 14, 2016 at 9:40 PM, olli hauer <oh...@gmx.de> wrote:
>>> I've done a quick test with
>>>  $ ab -n 10000 -c 100 $host/$url
>>>
>>> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...
>>
>> This looks normal to me, did the bahaviour changed w.r.t. 2.4.19?
>>
>> When the stress is running, the number of idlers (workers that do
>> nothing) fluctuates, according to the number of connections/requests
>> and children created to sustain the load.
>>
>> When the load stops, the number of idlers increases, and won't
>> decrease unless MaxSpareThreads is reached (i.e. children are stopped,
>> each releasing ThreadsPerChild idlers).
>>
>
> Ah, OK this will explain the numbers.
> Unluckily I haven't looked to scoreboard with 2.4.19, but give me some minutes ...

Could you also confirm with 2.4.20 + my patch that the Client, VHost
and Request colomns are always filled with latest values (not empty)
for idle workers (used at least once)?

This requires "ExtendedStatus on" (or default) for the corresponding
table to be displayed by mod_status.

Thanks for testing!

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Apr 14, 2016 at 10:58 PM, olli hauer <oh...@gmx.de> wrote:
>
> OK, I've attached the output from a 2.4.18 scoreboard some sec. after
>  ab -k -n 100000 -c 100 -f TLS1.2 $host/$url
>
> the 2.4.18 scoreboard looks similar to 2.4.20 with your patch

Thank you very much olli.

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by olli hauer <oh...@gmx.de>.
On 2016-04-14 22:43, Yann Ylavic wrote:
> On Thu, Apr 14, 2016 at 10:28 PM, olli hauer <oh...@gmx.de> wrote:
>>
>> I've done some tests with 2.4.19 there is maybe an interesting detail.
>> With 2.4.19 the last request is empty for an idle worker, with 2.4.20 not (shows the client, proto, Vhost and request)
> 
> Sorry for the confusion, 2.4.19 (not released) contained the change
> already, the relevant version to compare with is 2.4.18...
> 

OK, I've attached the output from a 2.4.18 scoreboard some sec. after
 ab -k -n 100000 -c 100 -f TLS1.2 $host/$url

the 2.4.18 scoreboard looks similar to 2.4.20 with your patch

-- 
olli

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Apr 14, 2016 at 10:28 PM, olli hauer <oh...@gmx.de> wrote:
>
> I've done some tests with 2.4.19 there is maybe an interesting detail.
> With 2.4.19 the last request is empty for an idle worker, with 2.4.20 not (shows the client, proto, Vhost and request)

Sorry for the confusion, 2.4.19 (not released) contained the change
already, the relevant version to compare with is 2.4.18...

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by olli hauer <oh...@gmx.de>.
On 2016-04-14 22:05, Rainer Jung wrote:
> Am 14.04.2016 um 22:05 schrieb olli hauer:
>> On 2016-04-14 21:48, Yann Ylavic wrote:
>>> On Thu, Apr 14, 2016 at 9:40 PM, olli hauer <oh...@gmx.de> wrote:
>>>> I've done a quick test with
>>>>   $ ab -n 10000 -c 100 $host/$url
>>>>
>>>> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...
>>>
>>> This looks normal to me, did the bahaviour changed w.r.t. 2.4.19?
>>>
>>> When the stress is running, the number of idlers (workers that do
>>> nothing) fluctuates, according to the number of connections/requests
>>> and children created to sustain the load.
>>>
>>> When the load stops, the number of idlers increases, and won't
>>> decrease unless MaxSpareThreads is reached (i.e. children are stopped,
>>> each releasing ThreadsPerChild idlers).
>>>
>>
>> Ah, OK this will explain the numbers.
>> Unluckily I haven't looked to scoreboard with 2.4.19, but give me some minutes ...
> 
> Not that important for the current discussion, but please note that "ab" does not use Keep-Alive connections by default. If you want to use HTTP Keep-Alive, add "-k".
> 
> Without it you create many new connections per second (depending on throughput this can easily be more than 1000 connections per second) which are only used very shortly (one request) and after shutting down they might end up in TIME_WAIT and stay there e.g. for a minute depending on OS details (you can check via "netstat"). Once you get more than a few thousand connections in TIME_WAIT, TCP/IP could get slower. Thus "ab -k" will often result in more stable behavior.
> 

Hi Rainer,

Thanks for the hint!
(indeed without -k I can simulate the svnstat tool that pulled my svn servers down)

I've done some tests with 2.4.19 there is maybe an interesting detail.
With 2.4.19 the last request is empty for an idle worker, with 2.4.20 not (shows the client, proto, Vhost and request)

I've attached the results as .txt file.

-- 
olli

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Rainer Jung <ra...@kippdata.de>.
Am 14.04.2016 um 22:05 schrieb olli hauer:
> On 2016-04-14 21:48, Yann Ylavic wrote:
>> On Thu, Apr 14, 2016 at 9:40 PM, olli hauer <oh...@gmx.de> wrote:
>>> I've done a quick test with
>>>   $ ab -n 10000 -c 100 $host/$url
>>>
>>> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...
>>
>> This looks normal to me, did the bahaviour changed w.r.t. 2.4.19?
>>
>> When the stress is running, the number of idlers (workers that do
>> nothing) fluctuates, according to the number of connections/requests
>> and children created to sustain the load.
>>
>> When the load stops, the number of idlers increases, and won't
>> decrease unless MaxSpareThreads is reached (i.e. children are stopped,
>> each releasing ThreadsPerChild idlers).
>>
>
> Ah, OK this will explain the numbers.
> Unluckily I haven't looked to scoreboard with 2.4.19, but give me some minutes ...

Not that important for the current discussion, but please note that "ab" 
does not use Keep-Alive connections by default. If you want to use HTTP 
Keep-Alive, add "-k".

Without it you create many new connections per second (depending on 
throughput this can easily be more than 1000 connections per second) 
which are only used very shortly (one request) and after shutting down 
they might end up in TIME_WAIT and stay there e.g. for a minute 
depending on OS details (you can check via "netstat"). Once you get more 
than a few thousand connections in TIME_WAIT, TCP/IP could get slower. 
Thus "ab -k" will often result in more stable behavior.

Regards,

Rainer


Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by olli hauer <oh...@gmx.de>.
On 2016-04-14 21:48, Yann Ylavic wrote:
> On Thu, Apr 14, 2016 at 9:40 PM, olli hauer <oh...@gmx.de> wrote:
>> I've done a quick test with
>>  $ ab -n 10000 -c 100 $host/$url
>>
>> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...
> 
> This looks normal to me, did the bahaviour changed w.r.t. 2.4.19?
> 
> When the stress is running, the number of idlers (workers that do
> nothing) fluctuates, according to the number of connections/requests
> and children created to sustain the load.
> 
> When the load stops, the number of idlers increases, and won't
> decrease unless MaxSpareThreads is reached (i.e. children are stopped,
> each releasing ThreadsPerChild idlers).
> 

Ah, OK this will explain the numbers.
Unluckily I haven't looked to scoreboard with 2.4.19, but give me some minutes ...

-- 
olli

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Apr 14, 2016 at 9:40 PM, olli hauer <oh...@gmx.de> wrote:
> I've done a quick test with
>  $ ab -n 10000 -c 100 $host/$url
>
> During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...

This looks normal to me, did the bahaviour changed w.r.t. 2.4.19?

When the stress is running, the number of idlers (workers that do
nothing) fluctuates, according to the number of connections/requests
and children created to sustain the load.

When the load stops, the number of idlers increases, and won't
decrease unless MaxSpareThreads is reached (i.e. children are stopped,
each releasing ThreadsPerChild idlers).

Regards,
Yann.

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by olli hauer <oh...@gmx.de>.
On 2016-04-14 20:36, Yann Ylavic wrote:
> On Thu, Apr 14, 2016 at 7:10 PM, olli hauer <oh...@gmx.de> wrote:
>> I got this morning a request from a FreeBSD user to back-port r1739008, and also already the feedback that scoreboard is usable again with the patch.
>> 
>> Since I cannot find a reference in branches/2.4.x/STATUS for r1739008 I told the user I will ask additional upstream devs if they think it is OK to include the patch until a new httpd release will be shipped.
> 
> r1739008+r1739146+r1739151 proposed for backport (full patch is [1]). Let's wait for votes...
> 
> Regards, Yann.
> 
> [1] http://home.apache.org/~ylavic/patches/httpd-2.4.x-scoreboard_preserve.patch
> 

Hi Yann,

thanks for the link!

I've done a quick test with
 $ ab -n 10000 -c 100 $host/$url

During the test the count of idle worker are incrementing and decrementing but it ab has finished the requests the count of idle workers stays on the last highest count ...


httpd some seconds after start:
---------------------------------------------
Server Version: Apache/2.4.20 (FreeBSD) OpenSSL/1.0.2g SVN/1.8.15 mod_wsgi/4.4.22 Python/2.7.11
Server MPM: event
Server Built: Apr 14 2016 21:09:43

Current Time: Thursday, 14-Apr-2016 21:22:13 CEST
Restart Time: Thursday, 14-Apr-2016 21:22:09 CEST
Parent Server Config. Generation: 1
Parent Server MPM Generation: 0
Server uptime: 4 seconds
Server load: 0.29 1.36 1.11
Total accesses: 0 - Total Traffic: 0 kB
CPU Usage: u0 s0 cu0 cs0
0 requests/sec - 0 B/second -
1 requests currently being processed, 74 idle workers

PID	Connections 	Threads	Async connections
total	accepting	busy	idle	writing	keep-alive	closing
31501	0	yes	0	25	0	0	0
31502	0	yes	1	24	0	0	0
31503	0	yes	0	25	0	0	0
Sum	0	 	1	74	0	0	0


after some manual requests:
---------------------------------------------
Current Time: Thursday, 14-Apr-2016 21:22:48 CEST
Restart Time: Thursday, 14-Apr-2016 21:22:09 CEST
Parent Server Config. Generation: 1
Parent Server MPM Generation: 0
Server uptime: 39 seconds
Server load: 0.28 1.24 1.08
Total accesses: 5 - Total Traffic: 18 kB
CPU Usage: u.0234375 s0 cu0 cs0 - .0601% CPU load
.128 requests/sec - 472 B/second - 3686 B/request
1 requests currently being processed, 99 idle workers

PID	Connections 	Threads	Async connections
total	accepting	busy	idle	writing	keep-alive	closing
31501	0	yes	0	25	0	0	0
31502	1	yes	1	24	0	0	0
31503	0	yes	0	25	0	0	0
31505	0	yes	0	25	0	0	0
Sum	1	 	1	99	0	0	0


give some load with ab -n 10000 -c 100
---------------------------------------------
Current Time: Thursday, 14-Apr-2016 21:23:29 CEST
Restart Time: Thursday, 14-Apr-2016 21:22:09 CEST
Parent Server Config. Generation: 1
Parent Server MPM Generation: 0
Server uptime: 1 minute 20 seconds
Server load: 1.49 1.38 1.13
Total accesses: 3315 - Total Traffic: 923 kB
CPU Usage: u36.7969 s.570313 cu0 cs0 - 46.7% CPU load
41.4 requests/sec - 11.5 kB/second - 285 B/request
86 requests currently being processed, 89 idle workers

PID	Connections 	Threads	Async connections
total	accepting	busy	idle	writing	keep-alive	closing
31501	3	yes	1	24	0	0	0
31502	2	yes	0	25	0	0	1
31503	25	yes	25	0	0	0	1
31505	25	yes	21	4	0	0	1
31579	2	yes	23	2	0	0	0
31580	3	yes	14	11	0	0	0
31581	3	yes	2	23	0	0	1
Sum	63	 	86	89	0	0	4


During the test the idle workers are going up and down but stay on the highest value after the test is finished.


ab requests finished, wait some time ...
------------------------------------------------------
Current Time: Thursday, 14-Apr-2016 21:29:12 CEST
Restart Time: Thursday, 14-Apr-2016 21:22:09 CEST
Parent Server Config. Generation: 1
Parent Server MPM Generation: 0
Server uptime: 7 minutes 3 seconds
Server load: 0.20 0.71 0.92
Total accesses: 10024 - Total Traffic: 2.9 MB
CPU Usage: u109.984 s1.4375 cu0 cs0 - 26.3% CPU load
23.7 requests/sec - 7.0 kB/second - 302 B/request
1 requests currently being processed, 174 idle workers

PID	Connections 	Threads	Async connections
total	accepting	busy	idle	writing	keep-alive	closing
31501	0	yes	0	25	0	0	0
31502	0	yes	0	25	0	0	0
31503	0	yes	0	25	0	0	0
31505	0	yes	0	25	0	0	0
31579	0	yes	0	25	0	0	0
31580	0	yes	0	25	0	0	0
31581	1	yes	1	24	0	1	0
Sum	1	 	1	174	0	1	0


running ab again (snapshot during the requests)
------------------------------------------------------
Restart Time: Thursday, 14-Apr-2016 21:22:09 CEST
Parent Server Config. Generation: 1
Parent Server MPM Generation: 0
Server uptime: 8 minutes 34 seconds
Server load: 4.30 1.53 1.19
Total accesses: 16090 - Total Traffic: 4.4 MB
CPU Usage: u176.695 s2.39063 cu0 cs0 - 34.8% CPU load
31.3 requests/sec - 8.7 kB/second - 284 B/request
98 requests currently being processed, 77 idle workers

PID	Connections 	Threads	Async connections
total	accepting	busy	idle	writing	keep-alive	closing
31501	0	yes	9	16	0	0	0
31502	2	yes	13	12	0	0	1
31503	13	yes	13	12	0	0	0
31505	13	yes	20	5	0	1	0
31579	0	yes	25	0	0	0	0
31580	3	yes	3	22	0	0	0
31581	6	yes	15	10	0	0	0
Sum	37	 	98	77	0	1	1


ab finished, wait some time ...
------------------------------------------------------

Current Time: Thursday, 14-Apr-2016 21:32:31 CEST
Restart Time: Thursday, 14-Apr-2016 21:22:09 CEST
Parent Server Config. Generation: 1
Parent Server MPM Generation: 0
Server uptime: 10 minutes 21 seconds
Server load: 1.31 1.46 1.21
Total accesses: 20033 - Total Traffic: 5.4 MB
CPU Usage: u219.664 s3.0625 cu0 cs0 - 35.9% CPU load
32.3 requests/sec - 8.9 kB/second - 282 B/request
1 requests currently being processed, 174 idle workers

PID	Connections 	Threads	Async connections
total	accepting	busy	idle	writing	keep-alive	closing
31501	0	yes	0	25	0	0	0
31502	0	yes	0	25	0	0	0
31503	0	yes	0	25	0	0	0
31505	0	yes	1	24	0	0	0
31579	0	yes	0	25	0	0	0
31580	0	yes	0	25	0	0	0
31581	0	yes	0	25	0	0	0
Sum	0	 	1	174	0	0	0


Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Apr 14, 2016 at 7:10 PM, olli hauer <oh...@gmx.de> wrote:
> I got this morning a request from a FreeBSD user to back-port r1739008, and
> also already the feedback that scoreboard is usable again with the patch.
>
> Since I cannot find a reference in branches/2.4.x/STATUS for r1739008 I told the
> user I will ask additional upstream devs if they think it is OK to include
> the patch until a new httpd release will be shipped.

r1739008+r1739146+r1739151 proposed for backport (full patch is [1]).
Let's wait for votes...

Regards,
Yann.

[1] http://home.apache.org/~ylavic/patches/httpd-2.4.x-scoreboard_preserve.patch

Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by olli hauer <oh...@gmx.de>.
On 2016-04-13 22:04, Yann Ylavic wrote:
> On Thu, Feb 25, 2016 at 11:27 AM,  <ic...@apache.org> wrote:
>> Author: icing
>> Date: Thu Feb 25 10:27:27 2016
>> New Revision: 1732275
>>
>> URL: http://svn.apache.org/viewvc?rev=1732275&view=rev
>> Log:
>> merging pre_close_connection hook, prep_lingering_close and ap_update_child() additions from trunk
>>
>> Modified:
> []
>>     httpd/httpd/branches/2.4.x/server/scoreboard.c
> []
...
> 
> This seems to have changed mod_status' output.
> Prior to this change the previous values were displayed until the
> worker was reused for another connection/request, now it is reset as
> soon as the worker is idle (i.e. each connection/request end).
> 
> I prefer the new behaviour, but there are some concerns on users@...
> 

I got this morning a request from a FreeBSD user to back-port r1739008, and
also already the feedback that scoreboard is usable again with the patch.

Since I cannot find a reference in branches/2.4.x/STATUS for r1739008 I told the
user I will ask additional upstream devs if they think it is OK to include
the patch until a new httpd release will be shipped.


Do you see any potential issues if r1739008 will be back-ported, or is there
additional work for a better patch in work?

-- 
olli

Re: Unintended Side Effects (Was: Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c)

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 14.04.2016 um 15:09 schrieb Jim Jagielski <ji...@jaguNET.com>:
> 
> We should always ensure that when we fix something, or
> when we add something, we don't change the current behavior
> of httpd in an unintended way. This rev is a great example of
> that. The clearing of those scoreboard slots is completely
> unrelated to the actual change itself.

Yes, I meant well, but should have known better or at least
asked for confirmation on the list. I have no excuse.

Scoreboard is one of the areas where unwanted changes can slip
through more easily than in other places, as there are no test
cases covering this text output.

Since its functionality is vital for a lot of users, maybe
we should invest into having a formatted, parseable variant
of its output. That we could use in tests.

> Of course, the "blame" is shared because even if the patch
> does change something, it is up to all of us to review it,
> not just by running and testing the code, but actually
> *inspecting* the patch for what it does.
> 
> The very fact that this change made it all the way
> thru the backport process to end up in 2.4.x is a reminder
> that we all need to stay vigilant.

Anything that is not covered by test code will break sooner
or later unless the original, sole author becomes immortal.

That is not meant as an excuse, just an observation.

-Stefan

Unintended Side Effects (Was: Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c)

Posted by Jim Jagielski <ji...@jaguNET.com>.
We should always ensure that when we fix something, or
when we add something, we don't change the current behavior
of httpd in an unintended way. This rev is a great example of
that. The clearing of those scoreboard slots is completely
unrelated to the actual change itself.

Of course, the "blame" is shared because even if the patch
does change something, it is up to all of us to review it,
not just by running and testing the code, but actually
*inspecting* the patch for what it does.

The very fact that this change made it all the way
thru the backport process to end up in 2.4.x is a reminder
that we all need to stay vigilant.


Re: svn commit: r1732275 - in /httpd/httpd/branches/2.4.x: ./ include/ap_mmn.h include/http_connection.h include/scoreboard.h modules/generators/mod_status.c modules/ssl/ssl_engine_kernel.c server/connection.c server/mpm/event/event.c server/scoreboard.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
The old behavior was expected, and very, very useful. It
was made that way for a reason.

+1 on reverting to old behavior...

> On Apr 13, 2016, at 4:04 PM, Yann Ylavic <yl...@gmail.com> wrote:
> 
> On Thu, Feb 25, 2016 at 11:27 AM,  <ic...@apache.org> wrote:
>> Author: icing
>> Date: Thu Feb 25 10:27:27 2016
>> New Revision: 1732275
>> 
>> URL: http://svn.apache.org/viewvc?rev=1732275&view=rev
>> Log:
>> merging pre_close_connection hook, prep_lingering_close and ap_update_child() additions from trunk
>> 
>> Modified:
> []
>>    httpd/httpd/branches/2.4.x/server/scoreboard.c
> []
>> 
>> Modified: httpd/httpd/branches/2.4.x/server/scoreboard.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/scoreboard.c?rev=1732275&r1=1732274&r2=1732275&view=diff
>> ==============================================================================
>> --- httpd/httpd/branches/2.4.x/server/scoreboard.c (original)
>> +++ httpd/httpd/branches/2.4.x/server/scoreboard.c Thu Feb 25 10:27:27 2016
>> @@ -32,6 +32,7 @@
>> #include "http_main.h"
>> #include "http_core.h"
>> #include "http_config.h"
>> +#include "http_protocol.h"
>> #include "ap_mpm.h"
>> 
>> #include "scoreboard.h"
>> @@ -457,7 +458,9 @@ static int update_child_status_internal(
>>                                         int thread_num,
>>                                         int status,
>>                                         conn_rec *c,
>> -                                        request_rec *r)
>> +                                        server_rec *s,
>> +                                        request_rec *r,
>> +                                        const char *descr)
>> {
>>     int old_status;
>>     worker_score *ws;
> []
>> @@ -489,28 +495,44 @@ static int update_child_status_internal(
>>             }
>>             ws->conn_count = 0;
>>             ws->conn_bytes = 0;
>> +            ws->last_used = apr_time_now();
>>         }
>> -        if (r) {
>> -            const char *client = ap_get_remote_host(c, r->per_dir_config,
>> -                                 REMOTE_NOLOOKUP, NULL);
>> -            if (!client || !strcmp(client, c->client_ip)) {
>> -                apr_cpystrn(ws->client, r->useragent_ip, sizeof(ws->client));
>> +        if (status == SERVER_READY) {
>> +            ws->client[0]='\0';
>> +            ws->vhost[0]='\0';
>> +            ws->request[0]='\0';
>> +            ws->protocol[0]='\0';
>> +        }
> 
> This seems to have changed mod_status' output.
> Prior to this change the previous values were displayed until the
> worker was reused for another connection/request, now it is reset as
> soon as the worker is idle (i.e. each connection/request end).
> 
> I prefer the new behaviour, but there are some concerns on users@...