Jeff Trawick wrote:

There are problems accounting for child processes which are trying to
initialize that result in the parent thinking it needs to create more
children.  The less harmful flavor is when it thinks (incorrectly) it
is already at MaxClients and issues the "reached MaxClients" message.

working on a patch to improve the message.

More disturbing is when MaxClients is very high and the parent keeps
creating new children using exponential ramp-up.  That can be very
painful.

  However, if the child processes are starting "slowly" because
ap_run_child_init() in child_main() is taking its time, then
start_threads() hasn't even been run yet, so the threads aren't
marked SERVER_STARTING -- they're just set to 0 as the default
value.  But 0 == SERVER_DEAD, so the main process sees a lot
of dead worker threads and begins spawning new child processes,
up to MaxClients/ThreadsPerChild in the worst case.

  I considered wedging another thread status into the
scoreboard, between SERVER_DEAD (the initial value) and
SERVER_STARTING.  The make_child() would set all the thread
slots to this value and start_threads() would later flip them
to SERVER_STARTING after actually creating the worker threads.

that would be the cleanest fix IMO. I dislike having state information scattered all over the place due to bad experiences with such in a previous job. whether or not it is practical to have a single state field in a stable httpd release is another question.

  That would have various ripple effects on other bits of
httpd, though, like mod_status and other MPMs, etc.sh

In other words, breaks binary compatibility...

so the binary state values in the scoreboard are an API? if we never declared they weren't an API maybe they are by default. but that really ties our hands. syncing mod_status and the event MPM doesn't bother me much.

Other modules should see the threads in SERVER_STARTING state anyway. IOW, I think we should set state to SERVER_STARTING before we do any
potentially-lengthy work like running child-init hooks so that the
state as seen from the outside makes sense.  That also means resetting
the state if something fails (e.g., pthread_create()).

But that isn't needed for proper operation of the MPM, which is what
we're after at the moment...  But it would be great to be able to see
from mod_status that a child is taking way too long in the
SERVER_STARTING state.

yep, or whatever we call this state.
                                                        So instead
I was considering adding something to process_score for this issue but
I decided against it, hopefully for an bogus reason -- binary
compatibility breakage.

This isn't binary compatibility breakage since we provide
ap_get_scoreboard_process() for modules to retrieve a process_score
structure, and if fields get added to the end for the use of the MPM
then no worries since we don't support modules creating their own
process_score structures and stuffing them in the scoreboard.

(confirmation from the crowd?)

Instead of "unsigned char status" I'd prefer something like

apr_int32_t mpm_state;   /* internal state for MPM; meaning may change
                                      * in the future, so not for use
by other modules
                                      */

adding something to the end of process score is probably the best solution for stable releases. but in trunk I would prefer to see: * the worker score state field be the one true state field for purposes of managing processes/threads to simplify logic + developer understanding,
* the preceding marked as internal, subject to change,
* an appropriate mmn bump (not an expert here),
* the state values spread out some with some reserved values interspersed.

the latter would let us un-reserve a state value in the right range if we need it in the future without as much potential disruption. the downside is that arrays indexed by state values would grow. no big deal.

I volunteer to do this work in trunk if we have consensus that it's the right 
thing.

If a particular MPM wants to store SERVER_STARTING/SERVER_DEAD/etc. then fine.


  During this period, while the new child process is running
ap_run_child_init() and friends, perform_idle_server_maintenance()
just counts that child process's worker threads as all being
effectively in SERVER_STARTING mode.  Once the process_score.status
field changes to SERVER_READY, perform_idle_server_maintenance()
begins to look at the individual thread status values.

  Any thoughts?  The patch in Bugzilla doesn't address other
MPMs that might see the same behaviour (event, and maybe prefork?)

http://issues.apache.org/bugzilla/show_bug.cgi?id=39275

A gracefully exiting process has lost its process score field and
gradually loses its worker_score fields as well.  Gracefully exiting
threads aren't counted as active or idle.

I think this means we can create a new process to make up for
gracefully exiting threads that we won't necessarily need once they
finish and new threads in that process scoreboard slot take over. Unavoidable, since gracefully exiting threads can take forever.

right. not counting gracefully exiting threads as idle helps p_i_s_m start new processes sooner.

Greg

Reply via email to