Amos Jeffries wrote:
Robert Collins wrote:
On Wed, 2009-07-15 at 00:52 +0200, Henrik Nordstrom wrote:
Or 4, go back to don't strictly enforce the number of helpers?
+1
I don't know what this strictly-enforce thing is, but it sounds unneeded
as we used to fire up the right number of helpers anyway.
I stopped Squid saying:
"running: 200 of 100 XXX helpers"
A Henrik said,
people with large memory-hog helpers have issues when Squid allocates
more than N bunches of their carefully tuned available memory to its
helpers. This is also important in low-memory systems requiring auth.
It's a simple 'start N' call now checks the number of running helpers
before blindly starting new ones. Making Squid actually follow its
numerous children=N settings.
I'm fine with reverting it in 3.1. But this is a nasty mix of sync and
async operations that does need cleaning up in 3.2. It's semi-hiding
about 4 bugs in a helpers and auth.
Right, following up on that I had a hunch and discovered that n_active
is already performing this duty. The bug is therefore in my earlier fix
using n_running as a base instead of n_active.
How does this new attached patch look to you two?
It performs the calculation re-base and documents the relevant counters.
Amos
=== modified file 'src/helper.cc'
--- src/helper.cc 2009-06-14 12:35:17 +0000
+++ src/helper.cc 2009-07-16 10:53:22 +0000
@@ -103,7 +103,7 @@
shortname = xstrdup(progname);
/* dont ever start more than hlp->n_to_start processes. */
- int need_new = hlp->n_to_start - hlp->n_running;
+ int need_new = hlp->n_to_start - hlp->n_active;
debugs(84, 1, "helperOpenServers: Starting " << need_new << "/" << hlp->n_to_start << " '" << shortname << "' processes");
@@ -209,7 +209,8 @@
shortname = xstrdup(progname);
/* dont ever start more than hlp->n_to_start processes. */
- int need_new = hlp->n_to_start - hlp->n_running;
+ /* n_active are the helpers which have not been shut down. */
+ int need_new = hlp->n_to_start - hlp->n_active;
debugs(84, 1, "helperOpenServers: Starting " << need_new << "/" << hlp->n_to_start << " '" << shortname << "' processes");
@@ -545,8 +546,8 @@
storeAppendPrintf(sentry, "program: %s\n",
hlp->cmdline->key);
- storeAppendPrintf(sentry, "number running: %d of %d\n",
- hlp->n_running, hlp->n_to_start);
+ storeAppendPrintf(sentry, "number active: %d of %d (%d shutting down)\n",
+ hlp->n_active, hlp->n_to_start, (hlp->n_running - hlp->n_active) );
storeAppendPrintf(sentry, "requests sent: %d\n",
hlp->stats.requests);
storeAppendPrintf(sentry, "replies received: %d\n",
@@ -587,7 +588,7 @@
storeAppendPrintf(sentry, " B = BUSY\n");
storeAppendPrintf(sentry, " W = WRITING\n");
storeAppendPrintf(sentry, " C = CLOSING\n");
- storeAppendPrintf(sentry, " S = SHUTDOWN\n");
+ storeAppendPrintf(sentry, " S = SHUTDOWN PENDING\n");
}
void
@@ -598,8 +599,8 @@
storeAppendPrintf(sentry, "program: %s\n",
hlp->cmdline->key);
- storeAppendPrintf(sentry, "number running: %d of %d\n",
- hlp->n_running, hlp->n_to_start);
+ storeAppendPrintf(sentry, "number active: %d of %d (%d shutting down)\n",
+ hlp->n_active, hlp->n_to_start, (hlp->n_running - hlp->n_active) );
storeAppendPrintf(sentry, "requests sent: %d\n",
hlp->stats.requests);
storeAppendPrintf(sentry, "replies received: %d\n",
@@ -644,7 +645,7 @@
storeAppendPrintf(sentry, " B = BUSY\n");
storeAppendPrintf(sentry, " C = CLOSING\n");
storeAppendPrintf(sentry, " R = RESERVED or DEFERRED\n");
- storeAppendPrintf(sentry, " S = SHUTDOWN\n");
+ storeAppendPrintf(sentry, " S = SHUTDOWN PENDING\n");
storeAppendPrintf(sentry, " P = PLACEHOLDER\n");
}
@@ -671,7 +672,6 @@
hlp->n_active--;
assert(hlp->n_active >= 0);
-
srv->flags.shutdown = 1; /* request it to shut itself down */
if (srv->flags.closing) {
=== modified file 'src/helper.h'
--- src/helper.h 2009-04-07 13:16:59 +0000
+++ src/helper.h 2009-07-16 10:51:42 +0000
@@ -58,9 +58,9 @@
dlink_list servers;
dlink_list queue;
const char *id_name;
- int n_to_start;
- int n_running;
- int n_active;
+ int n_to_start; ///< Configuration setting of how many helper children should be running
+ int n_running; ///< Total helper children objects currently existing
+ int n_active; ///< Count of helper children active (not shutting down)
int ipc_type;
IpAddress addr;
unsigned int concurrency;
@@ -80,9 +80,9 @@
dlink_list servers;
dlink_list queue;
const char *id_name;
- int n_to_start;
- int n_running;
- int n_active;
+ int n_to_start; ///< Configuration setting of how many helper children should be running
+ int n_running; ///< Total helper children objects currently existing
+ int n_active; ///< Count of helper children active (not shutting down)
int ipc_type;
IpAddress addr;
MemAllocator *datapool;
