You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Bill Stoddard <bi...@wstoddard.com> on 2002/04/26 20:07:29 UTC
[PATCH] Don't accept more connections than workers
This is a patch to worker.c to prevent more connections from being accepted than there are
workers to handle them. The accept thread decrements the avail count and the workers
increment the avail count.
I don't have a linux box handy so i cannot confirm this patch compiles. Sorry.
Bill
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/worker/worker.c,v
retrieving revision 1.117
diff -u -r1.117 worker.c
--- worker.c 18 Apr 2002 17:46:20 -0000 1.117
+++ worker.c 26 Apr 2002 17:59:16 -0000
@@ -156,6 +156,7 @@
*/
int ap_threads_per_child = 0; /* Worker threads per child */
+static int worker_thread_cnt = 0;
static int ap_daemons_to_start = 0;
static int min_spare_threads = 0;
static int max_spare_threads = 0;
@@ -693,6 +694,14 @@
}
if (listener_may_exit) break;
+ /* If no worker threads are available, yield our quanta and try again
+ * later
+ */
+ if (!worker_thread_cnt) {
+ yield();
+ continue;
+ }
+
if ((rv = SAFE_ACCEPT(apr_proc_mutex_lock(accept_mutex)))
!= APR_SUCCESS) {
int level = APLOG_EMERG;
@@ -791,6 +800,7 @@
signal_threads(ST_GRACEFUL);
}
if (csd != NULL) {
+ worker_thread_cnt--;
rv = ap_queue_push(worker_queue, csd, ptrans,
&recycled_pool);
if (rv) {
@@ -852,6 +862,7 @@
while (!workers_may_exit) {
ap_update_child_status_from_indexes(process_slot, thread_slot, SERVER_READY,
NULL);
+ worker_thread_cnt++;
rv = ap_queue_pop(worker_queue, &csd, &ptrans, last_ptrans);
last_ptrans = NULL;
Re: [PATCH] Don't accept more connections than workers
Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Apr 26, 2002 at 02:53:59PM -0400, Cliff Woolley wrote:
> On Fri, 26 Apr 2002, Aaron Bannert wrote:
>
> > LIFO->FIFO change this morning and added a counter to keep track of the
> > number of elements in the queue. That can be used in a new function
> > that blocks until the queue is non-full.
>
> You mean non-empty, I presume.
unfull? was-full-and-isn't-any-more? My mind is elsewhere, but as soon
as it returns (this evening) I'll whip up a patch for this. :)
-aaron
Re: [PATCH] Don't accept more connections than workers
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 26 Apr 2002, Aaron Bannert wrote:
> LIFO->FIFO change this morning and added a counter to keep track of the
> number of elements in the queue. That can be used in a new function
> that blocks until the queue is non-full.
You mean non-empty, I presume.
--Cliff
Re: [PATCH] Don't accept more connections than workers
Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Apr 26, 2002 at 11:18:58AM -0700, Justin Erenkrantz wrote:
> How about using APR condition variables instead of yield()? That was
> how I was imagining implementing this. The listener would block
> until the queue is not full and then drop into the accept mutex
> rotation. I seem to remember something like this being in the
> original worker code, but I don't recall. -- justin
Yes, using cond vars is the correct way to do this. I committed the
LIFO->FIFO change this morning and added a counter to keep track of the
number of elements in the queue. That can be used in a new function
that blocks until the queue is non-full.
-aaron
Re: [PATCH] Don't accept more connections than workers
Posted by Justin Erenkrantz <je...@apache.org>.
On Fri, Apr 26, 2002 at 02:07:29PM -0400, Bill Stoddard wrote:
> This is a patch to worker.c to prevent more connections from being accepted than there are
> workers to handle them. The accept thread decrements the avail count and the workers
> increment the avail count.
How about using APR condition variables instead of yield()? That was
how I was imagining implementing this. The listener would block
until the queue is not full and then drop into the accept mutex
rotation. I seem to remember something like this being in the
original worker code, but I don't recall. -- justin
Re: [PATCH] Don't accept more connections than workers
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 26 Apr 2002, Justin Erenkrantz wrote:
> > Besides, isn't there a race condition here? Or are you assuming ++ and --
> > are atomic?
>
> IIRC, there's no guarantee for that. -- justin
Right. That's why I'm confused. :)
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] Don't accept more connections than workers
Posted by Justin Erenkrantz <je...@apache.org>.
On Fri, Apr 26, 2002 at 02:16:51PM -0400, Cliff Woolley wrote:
> On Fri, 26 Apr 2002, Bill Stoddard wrote:
>
> > This is a patch to worker.c to prevent more connections from being
> > accepted than there are workers to handle them. The accept thread
> > decrements the avail count and the workers increment the avail count. I
> > don't have a linux box handy so i cannot confirm this patch compiles.
> > Sorry.
>
> Besides, isn't there a race condition here? Or are you assuming ++ and --
> are atomic?
IIRC, there's no guarantee for that. -- justin
Re: [PATCH] Don't accept more connections than workers
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 26 Apr 2002, Bill Stoddard wrote:
> + * later
> + */
> + if (!worker_thread_cnt) {
> + yield();
> + continue;
> + }
> +
It compiles, but it doesn't link. No such thing as yield():
server/mpm/worker/.libs/libworker.al(worker.lo): In function
`listener_thread':
worker.lo(.text+0x8dd): undefined reference to `yield'
collect2: ld returned 1 exit status
make[1]: *** [httpd] Error 1
I would say it ought to be apr_thread_yield(), but I see the unix
implementation of that in APR is this:
void apr_thread_yield()
{
}
Now THAT'S nice. :(
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [PATCH] Don't accept more connections than workers
Posted by Brian Pane <br...@cnet.com>.
Bill Stoddard wrote:
>>On Fri, 26 Apr 2002, Bill Stoddard wrote:
>>
>>>This is a patch to worker.c to prevent more connections from being
>>>accepted than there are workers to handle them. The accept thread
>>>decrements the avail count and the workers increment the avail count. I
>>>don't have a linux box handy so i cannot confirm this patch compiles.
>>>Sorry.
>>>
>>Besides, isn't there a race condition here? Or are you assuming ++ and --
>>are atomic?
>>
>
>Yep, and it is a bad assumption.
>
The safest place to increment/decrement the count is probably around the
apr_thread_cond_wait() call in the fd-queue pop function. The worker thread
is holding the global mutex right before and right after that call.
--Brian
Re: [PATCH] Don't accept more connections than workers
Posted by Bill Stoddard <bi...@wstoddard.com>.
> On Fri, 26 Apr 2002, Bill Stoddard wrote:
>
> > This is a patch to worker.c to prevent more connections from being
> > accepted than there are workers to handle them. The accept thread
> > decrements the avail count and the workers increment the avail count. I
> > don't have a linux box handy so i cannot confirm this patch compiles.
> > Sorry.
>
> Besides, isn't there a race condition here? Or are you assuming ++ and --
> are atomic?
Yep, and it is a bad assumption.
Bill
Re: [PATCH] Don't accept more connections than workers
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 26 Apr 2002, Bill Stoddard wrote:
> This is a patch to worker.c to prevent more connections from being
> accepted than there are workers to handle them. The accept thread
> decrements the avail count and the workers increment the avail count. I
> don't have a linux box handy so i cannot confirm this patch compiles.
> Sorry.
Besides, isn't there a race condition here? Or are you assuming ++ and --
are atomic?
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA