You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by sf...@apache.org on 2016/08/20 20:48:13 UTC
svn commit: r1757031 - in /httpd/httpd/trunk:
docs/log-message-tags/next-number server/mpm/event/event.c
Author: sf
Date: Sat Aug 20 20:48:13 2016
New Revision: 1757031
URL: http://svn.apache.org/viewvc?rev=1757031&view=rev
Log:
mpm_event: don't re-use scoreboard slots that are still in use
This causes inconsistent data in the scoreboard (due to async
connections) and makes it difficult to determine what is going on.
Therefore it is not a useful fix for the scoreboard-full issues (PR
53555).
The consent on the dev list is that we should allocate/use more
scoreboard entries instead.
Modified:
httpd/httpd/trunk/docs/log-message-tags/next-number
httpd/httpd/trunk/server/mpm/event/event.c
Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1757031&r1=1757030&r2=1757031&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Sat Aug 20 20:48:13 2016
@@ -1 +1 @@
-3455
+3456
Modified: httpd/httpd/trunk/server/mpm/event/event.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1757031&r1=1757030&r2=1757031&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/event/event.c (original)
+++ httpd/httpd/trunk/server/mpm/event/event.c Sat Aug 20 20:48:13 2016
@@ -625,27 +625,6 @@ static void event_note_child_started(int
retained->my_generation, slot, MPM_CHILD_STARTED);
}
-static void event_note_child_lost_slot(int slot, pid_t newpid)
-{
- ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(00458)
- "pid %" APR_PID_T_FMT " taking over scoreboard slot from "
- "%" APR_PID_T_FMT "%s",
- newpid,
- ap_scoreboard_image->parent[slot].pid,
- ap_scoreboard_image->parent[slot].quiescing ?
- " (quiescing)" : "");
- ap_run_child_status(ap_server_conf,
- ap_scoreboard_image->parent[slot].pid,
- ap_scoreboard_image->parent[slot].generation,
- slot, MPM_CHILD_LOST_SLOT);
- /* Don't forget about this exiting child process, or we
- * won't be able to kill it if it doesn't exit by the
- * time the server is shut down.
- */
- ap_register_extra_mpm_process(ap_scoreboard_image->parent[slot].pid,
- ap_scoreboard_image->parent[slot].generation);
-}
-
static const char *event_get_name(void)
{
return "event";
@@ -2727,6 +2706,15 @@ static int make_child(server_rec * s, in
retained->max_daemons_limit = slot + 1;
}
+ if (ap_scoreboard_image->parent[slot].pid != 0) {
+ /* XXX replace with assert or remove ? */
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(03455)
+ "BUG: Scoreboard slot %d should be empty but is "
+ "in use by pid %" APR_PID_T_FMT,
+ slot, ap_scoreboard_image->parent[slot].pid);
+ return -1;
+ }
+
if (one_process) {
my_bucket = &all_buckets[0];
@@ -2780,13 +2768,6 @@ static int make_child(server_rec * s, in
return -1;
}
- if (ap_scoreboard_image->parent[slot].pid != 0) {
- /* This new child process is squatting on the scoreboard
- * entry owned by an exiting child process, which cannot
- * exit until all active requests complete.
- */
- event_note_child_lost_slot(slot, pid);
- }
ap_scoreboard_image->parent[slot].quiescing = 0;
ap_scoreboard_image->parent[slot].not_accepting = 0;
ap_scoreboard_image->parent[slot].bucket = bucket;
@@ -2817,7 +2798,6 @@ static void perform_idle_server_maintena
worker_score *ws;
process_score *ps;
int free_length = 0;
- int totally_free_length = 0;
int free_slots[MAX_SPAWN_RATE];
int last_non_dead = -1;
int total_non_dead = 0;
@@ -2833,7 +2813,7 @@ static void perform_idle_server_maintena
int child_threads_active = 0;
if (i >= retained->max_daemons_limit &&
- totally_free_length == retained->idle_spawn_rate[child_bucket]) {
+ 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
*/
@@ -2872,28 +2852,9 @@ static void perform_idle_server_maintena
}
}
active_thread_count += child_threads_active;
- if (any_dead_threads
- && totally_free_length < retained->idle_spawn_rate[child_bucket]
- && free_length < MAX_SPAWN_RATE / num_buckets
- && (!ps->pid /* no process in the slot */
- || ps->quiescing)) { /* or at least one is going away */
- if (all_dead_threads) {
- /* great! we prefer these, because the new process can
- * start more threads sooner. So prioritize this slot
- * by putting it ahead of any slots with active threads.
- *
- * first, make room by moving a slot that's potentially still
- * in use to the end of the array
- */
- free_slots[free_length] = free_slots[totally_free_length];
- free_slots[totally_free_length++] = i;
- }
- else {
- /* slot is still in use - back of the bus
- */
- free_slots[free_length] = i;
- }
- ++free_length;
+ if (!ps->pid && 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;
@@ -2991,7 +2952,6 @@ static void perform_idle_server_maintena
static void server_main_loop(int remaining_children_to_start, int num_buckets)
{
- ap_generation_t old_gen;
int child_slot;
apr_exit_why_e exitwhy;
int status, processed_status;
@@ -3055,24 +3015,12 @@ static void server_main_loop(int remaini
--remaining_children_to_start;
}
}
- else if (ap_unregister_extra_mpm_process(pid.pid, &old_gen) == 1) {
-
- event_note_child_killed(-1, /* already out of the scoreboard */
- pid.pid, old_gen);
- if (processed_status == APEXIT_CHILDSICK
- && old_gen == retained->my_generation) {
- /* resource shortage, minimize the fork rate */
- for (i = 0; i < num_buckets; i++) {
- retained->idle_spawn_rate[i] = 1;
- }
- }
#if APR_HAS_OTHER_CHILD
- }
else if (apr_proc_other_child_alert(&pid, APR_OC_REASON_DEATH,
status) == 0) {
/* handled */
-#endif
}
+#endif
else if (retained->is_graceful) {
/* Great, we've probably just lost a slot in the
* scoreboard. Somehow we don't know about this child.