On 4/12/22 2:08 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Apr 12 12:08:02 2022
> New Revision: 1899777
>
> URL: http://svn.apache.org/viewvc?rev=1899777&view=rev
> Log:
> mpm_event: Fix accounting of active/total processes on ungraceful restart.
>
> Children processes terminated by ap_{reclaim,relieve}_child_processes() were
> were not un-accounted for total_daemons and active_daemons, which was done in
> server_main_loop() only. This led to perform_idle_server_maintenance() thinking
> it was over the limit of children processes and never create new ones.
>
> Have this accounting right in event_note_child_{started,stopped}() which is
> called both at runtime and reload time.
>
> * server/mpm/event/event.c(struct event_retained_data):
> Rename field max_daemons_limit to max_daemon_used to better describe what
> it's about and to align with AP_MPMQ_MAX_DAEMON_USED.
>
> * server/mpm/event/event.c(event_note_child_stopped):
> Renamed from event_note_child_killed() to clarify that it's not only called
> when a child is killed (i.e. on restart) but whenever a child has stopped.
>
> * server/mpm/event/event.c(event_note_child_stopped):
> Move decrementing {active,total}_daemons and marking child's threads as
> SERVER_DEAD from server_main_loop() so that it's done both at runtime and
> reload time. Log the current number/state of daemons at APLOG_DEBUG level
> for each child stopped.
>
> * server/mpm/event/event.c(event_note_child_started):
> Move incrementing {active,total}_daemons from make_child() for symmetry,
> given that make_child() calls event_note_child_started(). Log the current
> number/state of daemons at APLOG_DEBUG level for each child started.
>
> * server/mpm/event/event.c(perform_idle_server_maintenance):
> Fix possible miscounting of retained->max_daemon_used accross the multiple
> calls to perform_idle_server_maintenance() if ListenCoresBucketsRatio > 0.
> Pass an int *max_daemon_used which starts at zero and is bumped consistently
> for all the buckets, while retained->max_daemon_used is updated only after
> all the buckets have been maintained.
>
> * server/mpm/event/event.c(perform_idle_server_maintenance):
> Use event_note_child_stopped() to handle exited children processes.
>
>
> Fixes: BZ 66004
>
>
> Modified:
> httpd/httpd/trunk/server/mpm/event/event.c
>
> Modified: httpd/httpd/trunk/server/mpm/event/event.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1899777&r1=1899776&r2=1899777&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Tue Apr 12 12:08:02 2022
> @@ -3447,9 +3480,11 @@ static void server_main_loop(int remaini
> continue;
> }
>
> + max_daemon_used = 0;
> for (i = 0; i < num_buckets; i++) {
> - perform_idle_server_maintenance(i);
> + perform_idle_server_maintenance(i, &max_daemon_used);
> }
> + retained->max_daemon_used = max_daemon_used;
Shouldn't we do the above only when max_daemon_used > retained->max_daemon_used ? Otherwise we might shrink
retained->max_daemon_used if make_child increased it?
Regards
Rüdiger
> Author: ylavic
> Date: Tue Apr 12 12:08:02 2022
> New Revision: 1899777
>
> URL: http://svn.apache.org/viewvc?rev=1899777&view=rev
> Log:
> mpm_event: Fix accounting of active/total processes on ungraceful restart.
>
> Children processes terminated by ap_{reclaim,relieve}_child_processes() were
> were not un-accounted for total_daemons and active_daemons, which was done in
> server_main_loop() only. This led to perform_idle_server_maintenance() thinking
> it was over the limit of children processes and never create new ones.
>
> Have this accounting right in event_note_child_{started,stopped}() which is
> called both at runtime and reload time.
>
> * server/mpm/event/event.c(struct event_retained_data):
> Rename field max_daemons_limit to max_daemon_used to better describe what
> it's about and to align with AP_MPMQ_MAX_DAEMON_USED.
>
> * server/mpm/event/event.c(event_note_child_stopped):
> Renamed from event_note_child_killed() to clarify that it's not only called
> when a child is killed (i.e. on restart) but whenever a child has stopped.
>
> * server/mpm/event/event.c(event_note_child_stopped):
> Move decrementing {active,total}_daemons and marking child's threads as
> SERVER_DEAD from server_main_loop() so that it's done both at runtime and
> reload time. Log the current number/state of daemons at APLOG_DEBUG level
> for each child stopped.
>
> * server/mpm/event/event.c(event_note_child_started):
> Move incrementing {active,total}_daemons from make_child() for symmetry,
> given that make_child() calls event_note_child_started(). Log the current
> number/state of daemons at APLOG_DEBUG level for each child started.
>
> * server/mpm/event/event.c(perform_idle_server_maintenance):
> Fix possible miscounting of retained->max_daemon_used accross the multiple
> calls to perform_idle_server_maintenance() if ListenCoresBucketsRatio > 0.
> Pass an int *max_daemon_used which starts at zero and is bumped consistently
> for all the buckets, while retained->max_daemon_used is updated only after
> all the buckets have been maintained.
>
> * server/mpm/event/event.c(perform_idle_server_maintenance):
> Use event_note_child_stopped() to handle exited children processes.
>
>
> Fixes: BZ 66004
>
>
> Modified:
> httpd/httpd/trunk/server/mpm/event/event.c
>
> Modified: httpd/httpd/trunk/server/mpm/event/event.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1899777&r1=1899776&r2=1899777&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Tue Apr 12 12:08:02 2022
> @@ -3447,9 +3480,11 @@ static void server_main_loop(int remaini
> continue;
> }
>
> + max_daemon_used = 0;
> for (i = 0; i < num_buckets; i++) {
> - perform_idle_server_maintenance(i);
> + perform_idle_server_maintenance(i, &max_daemon_used);
> }
> + retained->max_daemon_used = max_daemon_used;
Shouldn't we do the above only when max_daemon_used > retained->max_daemon_used ? Otherwise we might shrink
retained->max_daemon_used if make_child increased it?
Regards
Rüdiger