I also see:
/* kill off the idle ones */
- ap_mpm_pod_killpg(pod, retained->max_daemons_limit);
+ for (i = 0; i < num_buckets; i++) {
+ ap_mpm_pod_killpg(pod[i], i, retained->max_daemons_limit);
+ }
Is that right? Why isn't it: ap_mpm_pod_killpg(pod,
retained->max_daemons_limit, i); ??
/**
* Write data to the pipe-of-death, signalling that all child process
* should die.
* @param pod The pipe-of-death to write to.
* @param num The number of child processes to kill
+ * @param my_bucket the bucket that holds the dying child process.
*/
-AP_DECLARE(void) ap_mpm_pod_killpg(ap_pod_t *pod, int num);
+AP_DECLARE(void) ap_mpm_pod_killpg(ap_pod_t *pod, int num, int child_bucket);
Isn't 'num' the same in both implementation??
On May 31, 2014, at 12:03 PM, Jim Jagielski <[email protected]> wrote:
> Sorry I didn't catch this earlier:
>
> I see
>
> +++ httpd-trunk.new/include/mpm_common.h 2014-05-16 13:07:03.892987491
> -0400
> @@ -267,16 +267,18 @@
> * Write data to the pipe-of-death, signalling that one child process
> * should die.
> * @param pod the pipe-of-death to write to.
> + * @param my_bucket the bucket that holds the dying child process.
> */
> -AP_DECLARE(apr_status_t) ap_mpm_pod_signal(ap_pod_t *pod);
> +AP_DECLARE(apr_status_t) ap_mpm_pod_signal(ap_pod_t *pod, int child_bucket);
>
> We can change the API at this point. We could
> add another function, eg ap_mpm_pod_signal_ex() which
> takes the int param, but we can't modify ap_mpm_pod_signal()
> itself.
>
> On May 30, 2014, at 11:15 AM, Lu, Yingqi <[email protected]> wrote:
>
>> Thank you very much!
>>
>> Thanks,
>> Yingqi
>>
>> -----Original Message-----
>> From: Jim Jagielski [mailto:[email protected]]
>> Sent: Friday, May 30, 2014 7:07 AM
>> To: [email protected]
>> Subject: Re: [PATCH ASF bugzilla# 55897]prefork_mpm patch with SO_REUSEPORT
>> support
>>
>> Thx! Let me review. My plan is to fold into trunk this weekend.
>>
>> On May 16, 2014, at 2:53 PM, Lu, Yingqi <[email protected]> wrote:
>>
>>> Hi Jim,
>>>
>>> Thanks very much for clarifying this with me. I added #ifdef in the code to
>>> check _SC_NPROCESSORS_ONLN in the so_reuseport patch. Bucket patch does not
>>> use this parameter so that it remains the same.
>>>
>>> Attached are the two most recent patches. I already updated the bugzilla
>>> #55897 as well.
>>>
>>> Thanks,
>>> Yingqi
>>>
>>> -----Original Message-----
>>> From: Jim Jagielski [mailto:[email protected]]
>>> Sent: Thursday, May 15, 2014 7:53 AM
>>> To: [email protected]
>>> Subject: Re: [PATCH ASF bugzilla# 55897]prefork_mpm patch with
>>> SO_REUSEPORT support
>>>
>>> I was thinking more about the sysconf(_SC_NPROCESSORS_ONLN) stuff...
>>> We could either check for that during config/build or protect it with a
>>> #ifdef in the code (and create some logging so the admin nows if it was
>>> found or not).
>>>
>>> On May 14, 2014, at 11:59 AM, Lu, Yingqi <[email protected]> wrote:
>>>
>>>> Hi Jim,
>>>>
>>>> Thanks very much for your email.
>>>>
>>>> In the SO_REUSEPORT patch, SO_REUSEPORT support is checked inside listen.c
>>>> file. If the feature is not supported on the OS (for example, Linux kernel
>>>> < 3.9), it will fall back to the original behavior.
>>>>
>>>> In the bucket patch, there is no need to check the params. With single
>>>> listen statement, it is just the default behavior.
>>>>
>>>> Please let me know if this answers your question.
>>>>
>>>> Thanks,
>>>> Yingqi
>>>>
>>>> -----Original Message-----
>>>> From: Jim Jagielski [mailto:[email protected]]
>>>> Sent: Wednesday, May 14, 2014 6:57 AM
>>>> To: [email protected]
>>>> Subject: Re: [PATCH ASF bugzilla# 55897]prefork_mpm patch with
>>>> SO_REUSEPORT support
>>>>
>>>> This is very cool!
>>>>
>>>> mod_status assumes that sysconf() exists, but do we need to do a config
>>>> check on the params we use in these patches?
>>>> We look OK on Linux, FreeBSD and OSX...
>>>>
>>>> I'm +1 on folding into trunk.
>>>>
>>>> On May 13, 2014, at 7:55 PM, Lu, Yingqi <[email protected]> wrote:
>>>>
>>>>> Dear All,
>>>>>
>>>>> During the last couple weeks, I spent some time extending the original
>>>>> two patches from prefork MPM only to all three Linux MPMs (prefork,
>>>>> worker and event). Attached is the latest version of the two patches.
>>>>> Bugzilla database has also been updated already. The ID for the two
>>>>> patches are #55897 and #56279. Please refer to messages below for details
>>>>> on both of the patches.
>>>>>
>>>>> Quick test result on modern dual socket Intel platform (Linux Kernel
>>>>> 3.13.9) SO_REUSEPORT patch (bugzilla #55897)
>>>>> 1. Prefork MPM: 1 listen statement: 2.16X throughput improvement; 2
>>>>> listen statements: 2.33X throughput improvement
>>>>> 2. Worker MPM: 1 listen statement: 10% throughput improvement; 2
>>>>> listen statements: 35% throughput improvement
>>>>> 3. Event MPM: 1 listen statement: 13% throughput improvement; 2
>>>>> listen statements: throughput parity, but 62% response time reduction
>>>>> (with patch, 38% response time as original SW)
>>>>>
>>>>> Bucket patch (bugzilla #56279, only impact multiple listen statement case)
>>>>> 1. Prefork MPM: 2 listen statements: 42% throughput improvement
>>>>> 2. Worker MPM: 2 listen statements: 7% throughput improvement
>>>>>
>>>>> In all the above testing cases, significant response time reductions are
>>>>> observed, even with throughput improvements.
>>>>>
>>>>> Please let me know your feedback and comments.
>>>>>
>>>>> Thanks,
>>>>> Yingqi
>>>>> Software and workloads used in performance tests may have been optimized
>>>>> for performance only on Intel microprocessors. Performance tests, such as
>>>>> SYSmark and MobileMark, are measured using specific computer systems,
>>>>> components, software, operations and functions. Any change to any of
>>>>> those factors may cause the results to vary. You should consult other
>>>>> information and performance tests to assist you in fully evaluating your
>>>>> contemplated purchases, including the performance of that product when
>>>>> combined with other products.
>>>>>
>>>>> From: Lu, Yingqi [mailto:[email protected]]
>>>>> Sent: Monday, March 17, 2014 1:41 PM
>>>>> To: [email protected]
>>>>> Subject: RE: FW: [PATCH ASF bugzilla# 55897]prefork_mpm patch with
>>>>> SO_REUSEPORT support
>>>>>
>>>>> Dear all,
>>>>>
>>>>> Based on the feedback we received, we modified this patch. Here is the
>>>>> most recent version. We also modified the Bugzilla database(Bugzilla#
>>>>> 55897 for SO_REUSEPORT patch; Bugzilla# 56279 for bucket patch).
>>>>>
>>>>> Below are the changes we made into this new version:
>>>>>
>>>>> According to Yann Ylavic and other people's comments, we separate the
>>>>> original patch between with and without SO_REUSEPORT into two separated
>>>>> patches. The SO_REUSEPORT patch does not change the original listen
>>>>> sockets, it just duplicate the original one into multiple ones. Since the
>>>>> listen sockets are identical, there is no need to change the
>>>>> idle_server_maintenance function. The bucket patch (without
>>>>> SO_REUSEPORT), on the other hand, it breaks down the original listen
>>>>> record (if there are multiple listen socks) to multiple listen record
>>>>> linked lists. In this case, idle_server_maintenance is implemented at
>>>>> bucket level to address the situation that imbalanced traffic occurs
>>>>> among different listen sockets/children buckets. In the bucket patch, the
>>>>> polling in the child process is removed since each child only listens to
>>>>> 1 sock.
>>>>>
>>>>> According to Arkadiusz Miskiewicz's comment, we make the "detection of
>>>>> SO_REUSEPORT" at run time.
>>>>>
>>>>> According to Jeff Trawick's comments, 1. We generate the patches
>>>>> against the httpd trunk.
>>>>> 2. We tested the current patches and they do not impact event and worker
>>>>> mpms. If current patches can be accepted, we would be happy to extend
>>>>> them to other Linux based mpms. There are not much code changes, but
>>>>> require some time to setup the workload to test.
>>>>> 3. We removed unnecessary comments and changed APLOGNO(). We also changed
>>>>> some of the parameter/variable/function names to better represent their
>>>>> meanings.
>>>>> 4. There should be no build-in limitations for SO_REUSEPORT patch. For
>>>>> bucket patch, the only thing is the number of children bucket only scales
>>>>> to MAX_SPAWN_RATE. If there are more than 32 (current default
>>>>> MAX_SPQN_RATE) listen statements specified in the httpd.conf, the number
>>>>> of buckets will be fixed to 32. The reason for this is because that we
>>>>> implement the idle_server_maintenance at bucket level, each bucket's own
>>>>> max_spawn_rate is set to MAX_SPAWN_RATE/num_buckets.
>>>>>
>>>>> Again, thanks very much for all the comments and feedback. Please let us
>>>>> know if there are more changes we need to complete to make them accepted.
>>>>>
>>>>> Thanks,
>>>>> Yingqi Lu
>>>>>
>>>>>
>>>>>
>>>>> From: Lu, Yingqi
>>>>> Sent: Tuesday, March 04, 2014 10:43 AM
>>>>> To: [email protected]
>>>>> Subject: RE: FW: [PATCH ASF bugzilla# 55897]prefork_mpm patch with
>>>>> SO_REUSEPORT support
>>>>>
>>>>> Hi Jeff,
>>>>>
>>>>> Thanks very much for your time reviewing the patch! We will modify the
>>>>> patch according to your comments and repost it here.
>>>>>
>>>>> Thanks,
>>>>> Yingqi
>>>>>
>>>>> From: Jeff Trawick [mailto:[email protected]]
>>>>> Sent: Tuesday, March 04, 2014 10:08 AM
>>>>> To: Apache HTTP Server Development List
>>>>> Subject: Re: FW: [PATCH ASF bugzilla# 55897]prefork_mpm patch with
>>>>> SO_REUSEPORT support
>>>>>
>>>>> On Tue, Mar 4, 2014 at 10:35 AM, Lu, Yingqi <[email protected]> wrote:
>>>>> Hi All,
>>>>>
>>>>> I just want to ping again on this patch to gather your feedback and
>>>>> comments. Please refer to the messages below for patch details.
>>>>>
>>>>> If you need any additional information/supporting data, please let us
>>>>> know as well.
>>>>>
>>>>> Yeah, it has been on my todo list, but I don't have time to give an
>>>>> in depth review at the moment. Here are a few questions/comments.
>>>>> (And you'll have to deal with the fact that it is unnecessarily
>>>>> tedious for me to evaluate higher-level considerations if there are
>>>>> a lot of distractions, such as the code comments below ;) But
>>>>> others are of course free to chime in.)
>>>>>
>>>>> The patch should be against httpd trunk. It probably won't take much
>>>>> time for you to create that patch and confirm basic operation.
>>>>>
>>>>> What is the impact to other MPMs, even if they shouldn't use or don't
>>>>> have the necessary code to use SO_REUSEPORT at this time?
>>>>>
>>>>> Have you tried the event MPM?
>>>>>
>>>>> Is there a way for the admin to choose this behavior? Most won't care,
>>>>> but everyone's behavior is changed AFAICT.
>>>>>
>>>>> Are there built-in limitations in this patch that we should be aware of?
>>>>> E.g., the free slot/spawn rate changes suggest to me that there can't be
>>>>> more than 1025 children???
>>>>>
>>>>> We should assume for now that there's no reason this couldn't be
>>>>> committed to trunk after review/rework, so make sure it is as close as
>>>>> you can get it to what you think is the final form.
>>>>>
>>>>> For the configure-time check for 3.9 kernel: I think we'd also use
>>>>> AC_TRY_COMPILE at configure time to confirm that the SO_REUSEPORT
>>>>> definition is available, and not enable it if the system includes
>>>>> doesn't define it. (Does that cause a problem for any significant
>>>>> number of people?)
>>>>>
>>>>> Don't mention the patch in the patch ;) (e.g., "This function is
>>>>> added for the patch.")
>>>>>
>>>>> Incomplete comments on style/syntax issues:
>>>>>
>>>>> * mixing declarations and statements (e.g., "duplr->next = 0;
>>>>> apr_socket_t *temps;") isn't supported by all compilers and is
>>>>> distracting when reviewing
>>>>> * suitable identifier names (e.g., fix global variable "flag" and
>>>>> whatever else isn't appropriate; "ap_post_config_listeners" should
>>>>> be renamed to indicate what it does
>>>>> * APLOGNO(99999) and comments about fixing it: Instead put "APLOGNO()"
>>>>> and don't add reminders in comments
>>>>> * this doesn't seem portable: "int
>>>>> free_slots[MAX_SPAWN_RATE/num_buckets];"
>>>>> and so on
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Yingqi
>>>>>
>>>>>
>>>>> From: Lu, Yingqi
>>>>> Sent: Friday, January 24, 2014 3:26 PM
>>>>> To: [email protected]
>>>>> Subject: [PATCH ASF bugzilla# 55897]prefork_mpm patch with
>>>>> SO_REUSEPORT support
>>>>>
>>>>> Dear All,
>>>>>
>>>>> Our analysis of Apache httpd 2.4.7 prefork mpm, on 32 and 64 thread Intel
>>>>> Xeon 2600 series systems, using an open source three tier social
>>>>> networking web server workload, revealed performance scaling issues. In
>>>>> current software single listen statement (listen 80) provides better
>>>>> scalability due to un-serialized accept. However, when system is under
>>>>> very high load, this can lead to big number of child processes stuck in D
>>>>> state. On the other hand, the serialized accept approach cannot scale
>>>>> with the high load either. In our analysis, a 32-thread system, with 2
>>>>> listen statements specified, could scale to just 70% utilization, and a
>>>>> 64-thread system, with signal listen statement specified (listen 80, 4
>>>>> network interfaces), could scale to only 60% utilization.
>>>>>
>>>>> Based on those findings, we created a prototype patch for prefork mpm
>>>>> which extends performance and thread utilization. In Linux kernel newer
>>>>> than 3.9, SO_REUSEPORT is enabled. This feature allows multiple sockets
>>>>> listen to the same IP:port and automatically round robins connections. We
>>>>> use this feature to create multiple duplicated listener records of the
>>>>> original one and partition the child processes into buckets. Each bucket
>>>>> listens to 1 IP:port. In case of old kernel which does not have the
>>>>> SO_REUSEPORT enabled, we modified the "multiple listen statement case" by
>>>>> creating 1 listen record for each listen statement and partitioning the
>>>>> child processes into different buckets. Each bucket listens to 1 IP:port.
>>>>>
>>>>> Quick tests of the patch, running the same workload, demonstrated a 22%
>>>>> throughput increase with 32-threads system and 2 listen statements (Linux
>>>>> kernel 3.10.4). With the older kernel (Linux Kernel 3.8.8, without
>>>>> SO_REUSEPORT), 10% performance gain was measured. With single listen
>>>>> statement (listen 80) configuration, we observed over 2X performance
>>>>> improvements on modern dual socket Intel platforms (Linux Kernel 3.10.4).
>>>>> We also observed big reduction in response time, in addition to the
>>>>> throughput improvement gained in our tests 1.
>>>>>
>>>>> Following the feedback from the bugzilla website where we originally
>>>>> submitted the patch, we removed the dependency of APR change to simplify
>>>>> the patch testing process. Thanks Jeff Trawick for his good suggestion!
>>>>> We are also actively working on extending the patch to worker and event
>>>>> MPMs, as a next step. Meanwhile, we would like to gather comments from
>>>>> all of you on the current prefork patch. Please take some time test it
>>>>> and let us know how it works in your environment.
>>>>>
>>>>> This is our first patch to the Apache community. Please help us review it
>>>>> and let us know if there is anything we might revise to improve it. Your
>>>>> feedback is very much appreciated.
>>>>>
>>>>> Configuration:
>>>>> <IfModule prefork.c>
>>>>> ListenBacklog 105384
>>>>> ServerLimit 105000
>>>>> MaxClients 1024
>>>>> MaxRequestsPerChild 0
>>>>> StartServers 64
>>>>> MinSpareServers 8
>>>>> MaxSpareServers 16
>>>>> </IfModule>
>>>>>
>>>>> 1. Software and workloads used in performance tests may have been
>>>>> optimized for performance only on Intel microprocessors. Performance
>>>>> tests, such as SYSmark and MobileMark, are measured using specific
>>>>> computer systems, components, software, operations and functions. Any
>>>>> change to any of those factors may cause the results to vary. You should
>>>>> consult other information and performance tests to assist you in fully
>>>>> evaluating your contemplated purchases, including the performance of that
>>>>> product when combined with other products.
>>>>>
>>>>> Thanks,
>>>>> Yingqi
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Born in Roswell... married an alien...
>>>>> http://emptyhammock.com/
>>>>> http://edjective.org/
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Born in Roswell... married an alien...
>>>>> http://emptyhammock.com/
>>>>> http://edjective.org/
>>>>>
>>>>> <httpd_trunk_so_reuseport.patch><httpd_trunk_bucket.patch>
>>>>
>>>
>>> <httpd_trunk_so_reuseport.patch><httpd_trunk_bucket.patch>
>>
>
