From aed896d44f9cf5248a0077c8c2c31b2c72f1b68f Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 15 Oct 2021 10:29:00 +0000 Subject: [PATCH 1/6] mpm_event: Restart stopping of idle children after a load peak. PR 65626. r1770752 added an heuristic to avoid stopping children when the load triggers MaxSpareThreads but children take some time to shut down until the point where active_daemons_limit/ServerLimit is reached (scoreboard full) and no child gets created to handle incoming connections. However when this happens there is nothing to stop children again when the load settles down (besides MaxRequestsPerChild, which may be 0) so let's restart to stop children again if/when idle_thread_count reaches max_workers / 4. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894285 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit 4f9e7cf5d7dda9011afee1b886f5c27eec77d148) --- .../event-kill_at_total_daemons_limit.txt | 2 ++ server/mpm/event/event.c | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 changes-entries/event-kill_at_total_daemons_limit.txt diff --git a/changes-entries/event-kill_at_total_daemons_limit.txt b/changes-entries/event-kill_at_total_daemons_limit.txt new file mode 100644 index 00000000000..007e9615e2f --- /dev/null +++ b/changes-entries/event-kill_at_total_daemons_limit.txt @@ -0,0 +1,2 @@ + *) mpm_event: Restart stopping of idle children after a load peak. PR 65626. + [Yann Ylavic, Ruediger Pluem] diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index dddff35da06..158ca8a22af 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -2877,7 +2877,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) * gracefully finishing processes may accumulate, filling up the * scoreboard. To avoid running out of scoreboard entries, we * don't shut down more processes when the total number of processes - * is high. + * is high, until there are more than max_workers/4 idle threads. * * XXX It would be nice if we could * XXX - kill processes without keepalive connections first @@ -2885,13 +2885,19 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) * XXX depending on server load, later be able to resurrect them * or kill them */ - if (retained->total_daemons <= active_daemons_limit && - retained->total_daemons < server_limit) { + if ((retained->total_daemons <= active_daemons_limit + && retained->total_daemons < server_limit) + /* The above test won't transition from true to false until a child + * exits by itself (i.e. MaxRequestsPerChild reached), so the below + * test makes sure that the situation unblocks when the load falls + * significantly (regardless of MaxRequestsPerChild, e.g. 0) */ + || idle_thread_count > max_workers/4 / num_buckets) { /* Kill off one child */ ap_mpm_podx_signal(all_buckets[child_bucket].pod, AP_MPM_PODX_GRACEFUL); retained->idle_spawn_rate[child_bucket] = 1; } else { + /* Still busy enough, don't kill */ ap_log_error(APLOG_MARK, APLOG_TRACE5, 0, ap_server_conf, "Not shutting down child: total daemons %d / " "active limit %d / ServerLimit %d", From e51d198a89ef73f8ef5ada315df5eccaaa1bcf50 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 15 Oct 2021 10:49:32 +0000 Subject: [PATCH 2/6] Follow up to r1894285: add idle_threads and max_workers to trace5. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894286 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit 04dac4fb10e51cd62a80a3e5352055536f1e255f) --- server/mpm/event/event.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 158ca8a22af..89d47f7904e 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -2900,9 +2900,10 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) /* Still busy enough, don't kill */ ap_log_error(APLOG_MARK, APLOG_TRACE5, 0, ap_server_conf, "Not shutting down child: total daemons %d / " - "active limit %d / ServerLimit %d", + "active limit %d / ServerLimit %d / " + "idle threads %d / max workers %d", retained->total_daemons, active_daemons_limit, - server_limit); + server_limit, idle_thread_count, max_workers); } } else if (idle_thread_count < min_spare_threads / num_buckets) { From 572ddb59a69021669915680fd44b4b66f9f66664 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 15 Oct 2021 13:22:11 +0000 Subject: [PATCH 3/6] mpm_{event,worker}: Really perform_idle_server_maintenance() per bucket. mpm_{event,worker}: No need to pass num_buckets to perform_idle_server_maintenance() and server_main_loop(). mpm_worker: the last used scoreboard slot depends on pid != 0 only. Align max_daemons_limit value on all MPMs. That's the number of scoreboard slots in use, so it shouldn't be negative. Fix off by one in OS/2 too. Submitted by: ylavic Github: closes #274 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894291 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit a870a5413071cae9f49ddbd8eccda2c133fa49fd) --- server/mpm/event/event.c | 56 +++++++++++++++++----------------- server/mpm/mpmt_os2/mpmt_os2.c | 6 ++-- server/mpm/prefork/prefork.c | 1 - server/mpm/worker/worker.c | 41 ++++++++++++++----------- 4 files changed, 54 insertions(+), 50 deletions(-) diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 89d47f7904e..95b44227297 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -2780,41 +2780,39 @@ static void startup_children(int number_to_start) } } -static void perform_idle_server_maintenance(int child_bucket, int num_buckets) +static void perform_idle_server_maintenance(int child_bucket) { - int i, j; + int num_buckets = retained->mpm->num_buckets; int idle_thread_count = 0; - worker_score *ws; process_score *ps; int free_length = 0; int free_slots[MAX_SPAWN_RATE]; int last_non_dead = -1; int active_thread_count = 0; + int i, j; for (i = 0; i < server_limit; ++i) { - /* Initialization to satisfy the compiler. It doesn't know - * that threads_per_child is always > 0 */ - int status = SERVER_DEAD; - int child_threads_active = 0; - int bucket = i % num_buckets; - + if (num_buckets > 1 && (i % num_buckets) != child_bucket) { + /* We only care about child_bucket in this call */ + continue; + } if (i >= retained->max_daemons_limit && free_length == retained->idle_spawn_rate[child_bucket]) { /* short cut if all active processes have been examined and * enough empty scoreboard slots have been found */ - break; } + ps = &ap_scoreboard_image->parent[i]; if (ps->pid != 0) { + int child_threads_active = 0; if (ps->quiescing == 1) { ps->quiescing = 2; active_daemons--; } for (j = 0; j < threads_per_child; j++) { - ws = &ap_scoreboard_image->servers[i][j]; - status = ws->status; + int status = ap_scoreboard_image->servers[i][j].status; /* We consider a starting server as idle because we started it * at least a cycle ago, and if it still hasn't finished starting @@ -2823,26 +2821,26 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) * This depends on the ordering of SERVER_READY and SERVER_STARTING. */ if (status <= SERVER_READY && !ps->quiescing && !ps->not_accepting - && ps->generation == retained->mpm->my_generation - && bucket == child_bucket) - { + && ps->generation == retained->mpm->my_generation) { ++idle_thread_count; } if (status >= SERVER_READY && status < SERVER_GRACEFUL) { ++child_threads_active; } } + active_thread_count += child_threads_active; + if (child_threads_active == threads_per_child) { + had_healthy_child = 1; + } last_non_dead = i; } - active_thread_count += child_threads_active; - if (!ps->pid - && bucket == child_bucket - && free_length < retained->idle_spawn_rate[child_bucket]) + else if (free_length < retained->idle_spawn_rate[child_bucket]) { free_slots[free_length++] = i; - else if (child_threads_active == threads_per_child) - had_healthy_child = 1; + } } + retained->max_daemons_limit = last_non_dead + 1; + if (retained->sick_child_detected) { if (had_healthy_child) { /* Assume this is a transient error, even though it may not be. Leave @@ -2851,6 +2849,10 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) */ retained->sick_child_detected = 0; } + else if (child_bucket < num_buckets - 1) { + /* check for had_healthy_child up to the last child bucket */ + return; + } else { /* looks like a basket case, as no child ever fully initialized; give up. */ @@ -2866,8 +2868,6 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } } - retained->max_daemons_limit = last_non_dead + 1; - if (idle_thread_count > max_spare_threads / num_buckets) { /* @@ -2907,7 +2907,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } } else if (idle_thread_count < min_spare_threads / num_buckets) { - if (active_thread_count >= max_workers) { + if (active_thread_count >= max_workers / num_buckets) { if (0 == idle_thread_count) { if (!retained->maxclients_reported) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(00484) @@ -2988,8 +2988,9 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } } -static void server_main_loop(int remaining_children_to_start, int num_buckets) +static void server_main_loop(int remaining_children_to_start) { + int num_buckets = retained->mpm->num_buckets; int child_slot; apr_exit_why_e exitwhy; int status, processed_status; @@ -3098,7 +3099,7 @@ static void server_main_loop(int remaining_children_to_start, int num_buckets) } for (i = 0; i < num_buckets; i++) { - perform_idle_server_maintenance(i, num_buckets); + perform_idle_server_maintenance(i); } } } @@ -3175,7 +3176,7 @@ static int event_run(apr_pool_t * _pconf, apr_pool_t * plog, server_rec * s) retained->mpm->mpm_state = AP_MPMQ_RUNNING; - server_main_loop(remaining_children_to_start, num_buckets); + server_main_loop(remaining_children_to_start); retained->mpm->mpm_state = AP_MPMQ_STOPPING; if (retained->mpm->shutdown_pending && retained->mpm->is_ungraceful) { @@ -3473,7 +3474,6 @@ static int event_pre_config(apr_pool_t * pconf, apr_pool_t * plog, if (!retained) { retained = ap_retained_data_create(userdata_key, sizeof(*retained)); retained->mpm = ap_unixd_mpm_get_retained_data(); - retained->max_daemons_limit = -1; if (retained->mpm->module_loads) { test_atomics = 1; } diff --git a/server/mpm/mpmt_os2/mpmt_os2.c b/server/mpm/mpmt_os2/mpmt_os2.c index 77fc3e7f05c..b3adb03adba 100644 --- a/server/mpm/mpmt_os2/mpmt_os2.c +++ b/server/mpm/mpmt_os2/mpmt_os2.c @@ -79,7 +79,7 @@ int ap_min_spare_threads = 0; int ap_max_spare_threads = 0; /* Keep track of a few interesting statistics */ -int ap_max_daemons_limit = -1; +int ap_max_daemons_limit = 0; /* volatile just in case */ static int volatile shutdown_pending; @@ -344,8 +344,8 @@ static void spawn_child(int slot) "error spawning child, slot %d", slot); } - if (ap_max_daemons_limit < slot) { - ap_max_daemons_limit = slot; + if (slot + 1 > ap_max_daemons_limit) { + ap_max_daemons_limit = slot + 1; } ap_scoreboard_image->parent[slot].pid = proc_rc.codeTerminate; diff --git a/server/mpm/prefork/prefork.c b/server/mpm/prefork/prefork.c index 1cbe542a6d4..0bdfd5ae743 100644 --- a/server/mpm/prefork/prefork.c +++ b/server/mpm/prefork/prefork.c @@ -1288,7 +1288,6 @@ static int prefork_pre_config(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp if (!retained) { retained = ap_retained_data_create(userdata_key, sizeof(*retained)); retained->mpm = ap_unixd_mpm_get_retained_data(); - retained->max_daemons_limit = -1; retained->idle_spawn_rate = 1; } retained->mpm->mpm_state = AP_MPMQ_STARTING; diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c index bd56f618bd8..5cf8ead94e1 100644 --- a/server/mpm/worker/worker.c +++ b/server/mpm/worker/worker.c @@ -1365,11 +1365,10 @@ static void startup_children(int number_to_start) } } -static void perform_idle_server_maintenance(int child_bucket, int num_buckets) +static void perform_idle_server_maintenance(int child_bucket) { - int i, j; + int num_buckets = retained->mpm->num_buckets; int idle_thread_count; - worker_score *ws; process_score *ps; int free_length; int totally_free_length = 0; @@ -1377,6 +1376,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) int last_non_dead; int total_non_dead; int active_thread_count = 0; + int i, j; /* initialize the free_list */ free_length = 0; @@ -1388,13 +1388,15 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) for (i = 0; i < ap_daemons_limit; ++i) { /* Initialization to satisfy the compiler. It doesn't know * that threads_per_child is always > 0 */ - int status = SERVER_DEAD; int any_dying_threads = 0; int any_dead_threads = 0; int all_dead_threads = 1; int child_threads_active = 0; - int bucket = i % num_buckets; + if (num_buckets > 1 && (i % num_buckets) != child_bucket) { + /* We only care about child_bucket in this call */ + continue; + } if (i >= retained->max_daemons_limit && totally_free_length == retained->idle_spawn_rate[child_bucket]) { /* short cut if all active processes have been examined and @@ -1404,8 +1406,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } ps = &ap_scoreboard_image->parent[i]; for (j = 0; j < threads_per_child; j++) { - ws = &ap_scoreboard_image->servers[i][j]; - status = ws->status; + int status = ap_scoreboard_image->servers[i][j].status; /* XXX any_dying_threads is probably no longer needed GLA */ any_dying_threads = any_dying_threads || @@ -1425,8 +1426,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) loop if no pid? not much else matters */ if (status <= SERVER_READY && !ps->quiescing && - ps->generation == retained->mpm->my_generation && - bucket == child_bucket) { + ps->generation == retained->mpm->my_generation) { ++idle_thread_count; } if (status >= SERVER_READY && status < SERVER_GRACEFUL) { @@ -1436,7 +1436,6 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } active_thread_count += child_threads_active; if (any_dead_threads - && bucket == child_bucket && totally_free_length < retained->idle_spawn_rate[child_bucket] && free_length < MAX_SPAWN_RATE / num_buckets && (!ps->pid /* no process in the slot */ @@ -1464,11 +1463,15 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } /* XXX if (!ps->quiescing) is probably more reliable GLA */ if (!any_dying_threads) { - last_non_dead = i; ++total_non_dead; } + if (ps->pid != 0) { + last_non_dead = i; + } } + retained->max_daemons_limit = last_non_dead + 1; + if (retained->sick_child_detected) { if (had_healthy_child) { /* Assume this is a transient error, even though it may not be. Leave @@ -1477,6 +1480,10 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) */ retained->sick_child_detected = 0; } + else if (child_bucket < num_buckets - 1) { + /* check for had_healthy_child up to the last child bucket */ + return; + } else { /* looks like a basket case, as no child ever fully initialized; give up. */ @@ -1492,8 +1499,6 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } } - retained->max_daemons_limit = last_non_dead + 1; - if (idle_thread_count > max_spare_threads / num_buckets) { /* Kill off one child */ ap_mpm_podx_signal(all_buckets[child_bucket].pod, @@ -1504,7 +1509,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) /* terminate the free list */ if (free_length == 0) { /* scoreboard is full, can't fork */ - if (active_thread_count >= ap_daemons_limit * threads_per_child) { + if (active_thread_count >= max_workers / num_buckets) { /* no threads are "inactive" - starting, stopping, etc. */ /* have we reached MaxRequestWorkers, or just getting close? */ if (0 == idle_thread_count) { @@ -1567,8 +1572,9 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets) } } -static void server_main_loop(int remaining_children_to_start, int num_buckets) +static void server_main_loop(int remaining_children_to_start) { + int num_buckets = retained->mpm->num_buckets; ap_generation_t old_gen; int child_slot; apr_exit_why_e exitwhy; @@ -1682,7 +1688,7 @@ static void server_main_loop(int remaining_children_to_start, int num_buckets) } for (i = 0; i < num_buckets; i++) { - perform_idle_server_maintenance(i, num_buckets); + perform_idle_server_maintenance(i); } } } @@ -1764,7 +1770,7 @@ static int worker_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s) apr_proc_mutex_defname()); retained->mpm->mpm_state = AP_MPMQ_RUNNING; - server_main_loop(remaining_children_to_start, num_buckets); + server_main_loop(remaining_children_to_start); retained->mpm->mpm_state = AP_MPMQ_STOPPING; if (retained->mpm->shutdown_pending && retained->mpm->is_ungraceful) { @@ -2019,7 +2025,6 @@ static int worker_pre_config(apr_pool_t *pconf, apr_pool_t *plog, if (!retained) { retained = ap_retained_data_create(userdata_key, sizeof(*retained)); retained->mpm = ap_unixd_mpm_get_retained_data(); - retained->max_daemons_limit = -1; } retained->mpm->mpm_state = AP_MPMQ_STARTING; if (retained->mpm->baton != retained) { From fbbbd5ba04a226bdb87e68698428438925f03bdb Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 3 Dec 2021 12:59:02 +0000 Subject: [PATCH 4/6] mpm_event: Retain active_daemons instead of resetting it on restart. Since active_daemons is tracked by perform_idle_server_maintenance() and decremented when a child gets quiescing or exits, clearing it on restart is actually an off by -total_daemons (of the old gen) after the first calls to perform_idle_server_maintenance(). Let perform_idle_server_maintenance() be the only one to update active_daemons by putting it in the retained struct to keep track accross restarts. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1895550 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit fcae3fd2aac563c056ca3ae8869ebdcc3bca2e4b) --- server/mpm/event/event.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 95b44227297..6053ca143e1 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -167,8 +167,6 @@ static int ap_daemons_to_start = 0; /* StartServers */ static int min_spare_threads = 0; /* MinSpareThreads */ static int max_spare_threads = 0; /* MaxSpareThreads */ static int active_daemons_limit = 0; /* MaxRequestWorkers / ThreadsPerChild */ -static int active_daemons = 0; /* workers that still active, i.e. are - not shutting down gracefully */ static int max_workers = 0; /* MaxRequestWorkers */ static int server_limit = 0; /* ServerLimit */ static int thread_limit = 0; /* ThreadLimit */ @@ -389,7 +387,10 @@ typedef struct event_retained_data { * Not kept up-to-date when shutdown is pending. */ int total_daemons; - + /* + * Workers that still active, i.e. are not shutting down gracefully. + */ + int active_daemons; /* * idle_spawn_rate is the number of children that will be spawned on the * next maintenance cycle if there aren't enough idle servers. It is @@ -2759,7 +2760,7 @@ static int make_child(server_rec * s, int slot, int bucket) ap_scoreboard_image->parent[slot].quiescing = 0; ap_scoreboard_image->parent[slot].not_accepting = 0; event_note_child_started(slot, pid); - active_daemons++; + retained->active_daemons++; retained->total_daemons++; return 0; } @@ -2809,7 +2810,7 @@ static void perform_idle_server_maintenance(int child_bucket) int child_threads_active = 0; if (ps->quiescing == 1) { ps->quiescing = 2; - active_daemons--; + retained->active_daemons--; } for (j = 0; j < threads_per_child; j++) { int status = ap_scoreboard_image->servers[i][j].status; @@ -2938,16 +2939,16 @@ static void perform_idle_server_maintenance(int child_bucket) if (free_length > retained->idle_spawn_rate[child_bucket]) { free_length = retained->idle_spawn_rate[child_bucket]; } - if (free_length + active_daemons > active_daemons_limit) { - if (active_daemons < active_daemons_limit) { - free_length = active_daemons_limit - active_daemons; + if (free_length + retained->active_daemons > active_daemons_limit) { + if (retained->active_daemons < active_daemons_limit) { + free_length = active_daemons_limit - retained->active_daemons; } else { ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, ap_server_conf, "server is at active daemons limit, spawning " "of %d children cancelled: %d/%d active, " "rate %d", free_length, - active_daemons, active_daemons_limit, + retained->active_daemons, active_daemons_limit, retained->idle_spawn_rate[child_bucket]); free_length = 0; } @@ -2960,14 +2961,14 @@ static void perform_idle_server_maintenance(int child_bucket) "spawning %d children, there are around %d idle " "threads, %d active children, and %d children " "that are shutting down", free_length, - idle_thread_count, active_daemons, + idle_thread_count, retained->active_daemons, retained->total_daemons); } for (i = 0; i < free_length; ++i) { ap_log_error(APLOG_MARK, APLOG_TRACE5, 0, ap_server_conf, "Spawning new child: slot %d active / " "total daemons: %d/%d", - free_slots[i], active_daemons, + free_slots[i], retained->active_daemons, retained->total_daemons); make_child(ap_server_conf, free_slots[i], child_bucket); } @@ -3045,7 +3046,7 @@ static void server_main_loop(int remaining_children_to_start) event_note_child_killed(child_slot, 0, 0); ps = &ap_scoreboard_image->parent[child_slot]; if (!ps->quiescing) - active_daemons--; + retained->active_daemons--; ps->quiescing = 0; /* NOTE: We don't dec in the (child_slot < 0) case! */ retained->total_daemons--; @@ -3306,8 +3307,6 @@ static int event_run(apr_pool_t * _pconf, apr_pool_t * plog, server_rec * s) "SIGHUP received. Attempting to restart"); } - active_daemons = 0; - return OK; } From caf4df457fa1d48185b80dd83707c5830214e9dc Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 3 Dec 2021 13:25:51 +0000 Subject: [PATCH 5/6] mpm_event: Follow up to r1894285: new MaxSpareThreads heuristics. When at MaxSpareThreads, instead of deferring the stop if we are close to active/server limit let's wait for the pending exits to complete. This way we always and accurately account for slow-to-exit processes to avoid filling up the scoreboard, whether at the limits or not. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1895553 13f79535-47bb-0310-9956-ffa450edef68 --- server/mpm/event/event.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 6053ca143e1..0214d56b4c3 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -2869,16 +2869,15 @@ static void perform_idle_server_maintenance(int child_bucket) } } - if (idle_thread_count > max_spare_threads / num_buckets) - { + if (idle_thread_count > max_spare_threads / num_buckets) { /* * Child processes that we ask to shut down won't die immediately * but may stay around for a long time when they finish their * requests. If the server load changes many times, many such * gracefully finishing processes may accumulate, filling up the * scoreboard. To avoid running out of scoreboard entries, we - * don't shut down more processes when the total number of processes - * is high, until there are more than max_workers/4 idle threads. + * don't shut down more processes if there are stopping ones + * already (i.e. active_daemons < total_daemons). * * XXX It would be nice if we could * XXX - kill processes without keepalive connections first @@ -2886,26 +2885,21 @@ static void perform_idle_server_maintenance(int child_bucket) * XXX depending on server load, later be able to resurrect them * or kill them */ - if ((retained->total_daemons <= active_daemons_limit - && retained->total_daemons < server_limit) - /* The above test won't transition from true to false until a child - * exits by itself (i.e. MaxRequestsPerChild reached), so the below - * test makes sure that the situation unblocks when the load falls - * significantly (regardless of MaxRequestsPerChild, e.g. 0) */ - || idle_thread_count > max_workers/4 / num_buckets) { - /* Kill off one child */ + int ignore = (retained->active_daemons < retained->total_daemons); + ap_log_error(APLOG_MARK, APLOG_TRACE5, 0, ap_server_conf, + "%shutting down one child: " + "active daemons %d / active limit %d / " + "total daemons %d / ServerLimit %d / " + "idle threads %d / max workers %d", + (ignore) ? "Not s" : "S", + retained->active_daemons, active_daemons_limit, + retained->total_daemons, server_limit, + idle_thread_count, max_workers); + if (!ignore) { ap_mpm_podx_signal(all_buckets[child_bucket].pod, AP_MPM_PODX_GRACEFUL); - retained->idle_spawn_rate[child_bucket] = 1; - } else { - /* Still busy enough, don't kill */ - ap_log_error(APLOG_MARK, APLOG_TRACE5, 0, ap_server_conf, - "Not shutting down child: total daemons %d / " - "active limit %d / ServerLimit %d / " - "idle threads %d / max workers %d", - retained->total_daemons, active_daemons_limit, - server_limit, idle_thread_count, max_workers); } + retained->idle_spawn_rate[child_bucket] = 1; } else if (idle_thread_count < min_spare_threads / num_buckets) { if (active_thread_count >= max_workers / num_buckets) { From f72f1b0817f41963f5d30d119016b38724862356 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Mon, 6 Dec 2021 17:12:28 +0000 Subject: [PATCH 6/6] mpm_event: Follow up to r1895553. We can still kill processes above MaxSpareThreads at every maintenance cycle unless there is not enough headromm in the scoreboard for a graceful restart. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1895630 13f79535-47bb-0310-9956-ffa450edef68 --- server/mpm/event/event.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 0214d56b4c3..c2d53ff0735 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -2877,7 +2877,8 @@ static void perform_idle_server_maintenance(int child_bucket) * gracefully finishing processes may accumulate, filling up the * scoreboard. To avoid running out of scoreboard entries, we * don't shut down more processes if there are stopping ones - * already (i.e. active_daemons < total_daemons). + * already (i.e. active_daemons != total_daemons) and not enough + * slack space in the scoreboard for a graceful restart. * * XXX It would be nice if we could * XXX - kill processes without keepalive connections first @@ -2885,20 +2886,25 @@ static void perform_idle_server_maintenance(int child_bucket) * XXX depending on server load, later be able to resurrect them * or kill them */ - int ignore = (retained->active_daemons < retained->total_daemons); + int do_kill = (retained->active_daemons == retained->total_daemons + || (server_limit - retained->total_daemons > + active_daemons_limit)); ap_log_error(APLOG_MARK, APLOG_TRACE5, 0, ap_server_conf, "%shutting down one child: " "active daemons %d / active limit %d / " "total daemons %d / ServerLimit %d / " "idle threads %d / max workers %d", - (ignore) ? "Not s" : "S", + (do_kill) ? "S" : "Not s", retained->active_daemons, active_daemons_limit, retained->total_daemons, server_limit, idle_thread_count, max_workers); - if (!ignore) { + if (do_kill) { ap_mpm_podx_signal(all_buckets[child_bucket].pod, AP_MPM_PODX_GRACEFUL); } + else { + /* Wait for dying daemon(s) to exit */ + } retained->idle_spawn_rate[child_bucket] = 1; } else if (idle_thread_count < min_spare_threads / num_buckets) {