httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Ames <grega...@apache.org>
Subject Re: [PATCH] #39275 MaxClients on startup [Was: Bug in 2.0.56-dev]
Date Wed, 12 Apr 2006 18:26:43 GMT
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

Mime
View raw message