From: Henrik Nordstrom Date: Sun, 2 Aug 2009 22:54:14 +0000 (+0200) Subject: Cut away the deferred helper state X-Git-Tag: SQUID_3_2_0_1~819^2~5 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d20ce97d2485243522542bcd8adb52839a3e8b54;p=thirdparty%2Fsquid.git Cut away the deferred helper state --- diff --git a/src/enums.h b/src/enums.h index 3184fada41..4ab1f7c6d0 100644 --- a/src/enums.h +++ b/src/enums.h @@ -370,15 +370,13 @@ typedef enum { typedef enum { S_HELPER_UNKNOWN, S_HELPER_RESERVE, - S_HELPER_RELEASE, - S_HELPER_DEFER + S_HELPER_RELEASE } stateful_helper_callback_t; /* stateful helper reservation info */ typedef enum { S_HELPER_FREE, /* available for requests */ - S_HELPER_RESERVED, /* in a reserved state - no active request, but state data in the helper shouldn't be disturbed */ - S_HELPER_DEFERRED /* available for requests, and at least one more will come from a previous caller with the server pointer */ + S_HELPER_RESERVED /* in a reserved state - no active request, but state data in the helper shouldn't be disturbed */ } stateful_helper_reserve_t; diff --git a/src/helper.cc b/src/helper.cc index 101f869409..65f889fbdf 100644 --- a/src/helper.cc +++ b/src/helper.cc @@ -63,9 +63,6 @@ static void helperStatefulKickQueue(statefulhelper * hlp); static void helperRequestFree(helper_request * r); static void helperStatefulRequestFree(helper_stateful_request * r); static void StatefulEnqueue(statefulhelper * hlp, helper_stateful_request * r); -static helper_stateful_request *StatefulServerDequeue(helper_stateful_server * srv); -static void StatefulServerEnqueue(helper_stateful_server * srv, helper_stateful_request * r); -static void helperStatefulServerKickQueue(helper_stateful_server * srv); static bool helperStartStats(StoreEntry *sentry, void *hlp, const char *label); @@ -257,9 +254,6 @@ helperStatefulOpenServers(statefulhelper * hlp) srv->hIpc = hIpc; srv->pid = pid; srv->flags.reserved = S_HELPER_FREE; - srv->deferred_requests = 0; - srv->stats.deferbyfunc = 0; - srv->stats.deferbycb = 0; srv->stats.submits = 0; srv->stats.releases = 0; srv->index = k; @@ -326,8 +320,7 @@ helperSubmit(helper * hlp, const char *buf, HLPCB * callback, void *data) debugs(84, 9, "helperSubmit: " << buf); } -/* lastserver = "server last used as part of a deferred or reserved - * request sequence" +/* lastserver = "server last used as part of a reserved request sequence" */ void helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, void *data, helper_stateful_server * lastserver) @@ -353,28 +346,11 @@ helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, v if ((buf != NULL) && lastserver) { debugs(84, 5, "StatefulSubmit with lastserver " << lastserver); - /* the queue doesn't count for this assert because queued requests - * have already gone through here and been tested. - * It's legal to have deferred_requests == 0 and queue entries - * and status of S_HELPEER_DEFERRED. - * BUT: It's not legal to submit a new request w/lastserver in - * that state. - */ - assert(!(lastserver->deferred_requests == 0 && - lastserver->flags.reserved == S_HELPER_DEFERRED)); - - if (lastserver->flags.reserved != S_HELPER_RESERVED) { - lastserver->stats.submits++; - lastserver->deferred_requests--; - } + assert(lastserver->flags.reserved == S_HELPER_RESERVED; + asser(!(lastserver->request)); - if (!(lastserver->request)) { - debugs(84, 5, "StatefulSubmit dispatching"); - helperStatefulDispatch(lastserver, r); - } else { - debugs(84, 5, "StatefulSubmit queuing"); - StatefulServerEnqueue(lastserver, r); - } + debugs(84, 5, "StatefulSubmit dispatching"); + helperStatefulDispatch(lastserver, r); } else { helper_stateful_server *srv; if ((srv = StatefulGetFirstAvailable(hlp))) { @@ -386,79 +362,6 @@ helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, v debugs(84, 9, "helperStatefulSubmit: placeholder: '" << r->placeholder << "', buf '" << buf << "'."); } -/* - * helperStatefulDefer - * - * find and add a deferred request to a helper - */ -helper_stateful_server * -helperStatefulDefer(statefulhelper * hlp) -{ - if (hlp == NULL) { - debugs(84, 3, "helperStatefulDefer: hlp == NULL"); - return NULL; - } - - debugs(84, 5, "helperStatefulDefer: Running servers " << hlp->n_running); - - if (hlp->n_running == 0) { - debugs(84, 1, "helperStatefulDefer: No running servers!. "); - return NULL; - } - - helper_stateful_server *rv = StatefulGetFirstAvailable(hlp); - - if (rv == NULL) { - /* - * all currently busy; loop through servers and find server - * with the shortest queue - */ - - for (dlink_node *n = hlp->servers.head; n != NULL; n = n->next) { - helper_stateful_server *srv = (helper_stateful_server *)n->data; - - if (srv->flags.reserved == S_HELPER_RESERVED) - continue; - - if (!srv->flags.shutdown) - continue; - - if ((hlp->IsAvailable != NULL) && (srv->data != NULL) && - !(hlp->IsAvailable(srv->data))) - continue; - - if ((rv != NULL) && (rv->deferred_requests < srv->deferred_requests)) - continue; - - rv = srv; - } - } - - if (rv == NULL) { - debugs(84, 1, "helperStatefulDefer: None available."); - return NULL; - } - - /* consistency check: - * when the deferred count is 0, - * submits + releases == deferbyfunc + deferbycb - * Or in english, when there are no deferred requests, the amount - * we have submitted to the queue or cancelled must equal the amount - * we have said we wanted to be able to submit or cancel - */ - if (rv->deferred_requests == 0) - assert(rv->stats.submits + rv->stats.releases == - rv->stats.deferbyfunc + rv->stats.deferbycb); - - rv->flags.reserved = S_HELPER_DEFERRED; - - rv->deferred_requests++; - - rv->stats.deferbyfunc++; - - return rv; -} - void helperStatefulReset(helper_stateful_server * srv) /* puts this helper back in the queue. the calling app is required to @@ -479,17 +382,12 @@ helperStatefulReset(helper_stateful_server * srv) srv->flags.busy = 0; - if (srv->queue.head) { - srv->flags.reserved = S_HELPER_DEFERRED; - helperStatefulServerKickQueue(srv); - } else { - srv->flags.reserved = S_HELPER_FREE; + srv->flags.reserved = S_HELPER_FREE; - if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) - srv->parent->OnEmptyQueue(srv->data); + if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) + srv->parent->OnEmptyQueue(srv->data); - helperStatefulKickQueue(hlp); - } + helperStatefulKickQueue(hlp); } /* @@ -498,9 +396,6 @@ helperStatefulReset(helper_stateful_server * srv) * helperStatefulReleaseServer tells the helper that whoever was * using it no longer needs its services. * - * If the state is S_HELPER_DEFERRED, decrease the deferred count. - * If the count goes to zero, then it can become S_HELPER_FREE. - * * If the state is S_HELPER_RESERVED, then it should always * become S_HELPER_FREE. */ @@ -513,19 +408,6 @@ helperStatefulReleaseServer(helper_stateful_server * srv) srv->stats.releases++; - if (srv->flags.reserved == S_HELPER_DEFERRED) { - assert(srv->deferred_requests); - srv->deferred_requests--; - if (srv->deferred_requests) { - debugs(0,0,HERE << "helperStatefulReleaseServer srv->deferred_requests=" << srv->deferred_requests); - return; - } - if (srv->queue.head) { - debugs(0,0,HERE << "helperStatefulReleaseServer srv->queue.head not NULL"); - return; - } - } - srv->flags.reserved = S_HELPER_FREE; if (srv->parent->OnEmptyQueue != NULL && srv->data) srv->parent->OnEmptyQueue(srv->data); @@ -610,12 +492,11 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label storeAppendPrintf(sentry, "avg service time: %d msec\n", hlp->stats.avg_svc_time); storeAppendPrintf(sentry, "\n"); - storeAppendPrintf(sentry, "%7s\t%7s\t%7s\t%11s\t%20s\t%s\t%7s\t%7s\t%7s\n", + storeAppendPrintf(sentry, "%7s\t%7s\t%7s\t%11s\t%s\t%7s\t%7s\t%7s\n", "#", "FD", "PID", "# Requests", - "# Deferred Requests", "Flags", "Time", "Offset", @@ -625,15 +506,14 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label helper_stateful_server *srv = (helper_stateful_server *)link->data; double tt = 0.001 * tvSubMsec(srv->dispatch_time, srv->flags.busy ? current_time : srv->answer_time); - storeAppendPrintf(sentry, "%7d\t%7d\t%7d\t%11d\t%20d\t%c%c%c%c%c\t%7.3f\t%7d\t%s\n", + storeAppendPrintf(sentry, "%7d\t%7d\t%7d\t%11d\t%c%c%c%c%c\t%7.3f\t%7d\t%s\n", srv->index + 1, srv->rfd, srv->pid, srv->stats.uses, - (int) srv->deferred_requests, srv->flags.busy ? 'B' : ' ', srv->flags.closing ? 'C' : ' ', - srv->flags.reserved == S_HELPER_RESERVED ? 'R' : (srv->flags.reserved == S_HELPER_DEFERRED ? 'D' : ' '), + srv->flags.reserved == S_HELPER_RESERVED ? 'R' : ' '), srv->flags.shutdown ? 'S' : ' ', srv->request ? (srv->request->placeholder ? 'P' : ' ') : ' ', tt < 0.0 ? 0.0 : tt, @@ -644,7 +524,7 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label storeAppendPrintf(sentry, "\nFlags key:\n\n"); storeAppendPrintf(sentry, " B = BUSY\n"); storeAppendPrintf(sentry, " C = CLOSING\n"); - storeAppendPrintf(sentry, " R = RESERVED or DEFERRED\n"); + storeAppendPrintf(sentry, " R = RESERVED\n"); storeAppendPrintf(sentry, " S = SHUTDOWN PENDING\n"); storeAppendPrintf(sentry, " P = PLACEHOLDER\n"); } @@ -757,11 +637,6 @@ helperStatefulShutdown(statefulhelper * hlp) continue; } - if (srv->deferred_requests) { - debugs(84, 3, "helperStatefulShutdown: " << hlp->id_name << " #" << srv->index + 1 << " has DEFERRED requests."); - continue; - } - srv->flags.closing = 1; #ifdef _SQUID_MSWIN_ @@ -1159,42 +1034,20 @@ helperStatefulHandleRead(int fd, char *buf, size_t len, comm_err_t flag, int xer case S_HELPER_RELEASE: /* helper finished with */ - if (!srv->deferred_requests && !srv->queue.head) { - srv->flags.reserved = S_HELPER_FREE; + srv->flags.reserved = S_HELPER_FREE; - if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) - srv->parent->OnEmptyQueue(srv->data); + if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) + srv->parent->OnEmptyQueue(srv->data); - debugs(84, 5, "StatefulHandleRead: releasing " << hlp->id_name << " #" << srv->index + 1); - } else { - srv->flags.reserved = S_HELPER_DEFERRED; - debugs(84, 5, "StatefulHandleRead: outstanding deferred requests on " << - hlp->id_name << " #" << srv->index + 1 << - ". reserving for deferred requests."); - } + debugs(84, 5, "StatefulHandleRead: releasing " << hlp->id_name << " #" << srv->index + 1); break; case S_HELPER_RESERVE: /* 'pin' this helper for the caller */ - if (!srv->queue.head) { - assert(srv->deferred_requests == 0); - srv->flags.reserved = S_HELPER_RESERVED; - debugs(84, 5, "StatefulHandleRead: reserving " << hlp->id_name << " #" << srv->index + 1); - } else { - fatal("StatefulHandleRead: Callback routine attempted to reserve a stateful helper with deferred requests. This can lead to deadlock.\n"); - } - - break; + srv->flags.reserved = S_HELPER_RESERVED; + debugs(84, 5, "StatefulHandleRead: reserving " << hlp->id_name << " #" << srv->index + 1); - case S_HELPER_DEFER: - /* the helper is still needed, but can - * be used for other requests in the meantime. - */ - srv->flags.reserved = S_HELPER_DEFERRED; - srv->deferred_requests++; - srv->stats.deferbycb++; - debugs(84, 5, "StatefulHandleRead: reserving " << hlp->id_name << " #" << srv->index + 1 << " for deferred requests."); break; default: @@ -1216,18 +1069,13 @@ helperStatefulHandleRead(int fd, char *buf, size_t len, comm_err_t flag, int xer tvSubMsec(srv->dispatch_time, current_time), hlp->stats.replies, REDIRECT_AV_FACTOR); - if (srv->flags.shutdown - && srv->flags.reserved == S_HELPER_FREE - && !srv->deferred_requests) { + if (srv->flags.shutdown && srv->flags.reserved == S_HELPER_FREE) { int wfd = srv->wfd; srv->wfd = -1; srv->flags.closing=1; comm_close(wfd); } else { - if (srv->queue.head) - helperStatefulServerKickQueue(srv); - else - helperStatefulKickQueue(hlp); + helperStatefulKickQueue(hlp); } } @@ -1292,31 +1140,6 @@ StatefulEnqueue(statefulhelper * hlp, helper_stateful_request * r) } -static void -StatefulServerEnqueue(helper_stateful_server * srv, helper_stateful_request * r) -{ - dlink_node *link = (dlink_node *)memAllocate(MEM_DLINK_NODE); - dlinkAddTail(r, link, &srv->queue); - /* TODO: warning if the queue on this server is more than X - * We don't check the queue size at the moment, because - * requests hitting here are deferrable - */ - /* hlp->stats.queue_size++; - * if (hlp->stats.queue_size < hlp->n_running) - * return; - * if (squid_curtime - hlp->last_queue_warn < 600) - * return; - * if (shutting_down || reconfiguring) - * return; - * hlp->last_queue_warn = squid_curtime; - * debugs(84, 0, "WARNING: All " << hlp->id_name << " processes are busy."); - * debugs(84, 0, "WARNING: " << hlp->stats.queue_size << " pending requests queued"); - * if (hlp->stats.queue_size > hlp->n_running * 2) - * fatalf("Too many queued %s requests", hlp->id_name); - * debugs(84, 1, "Consider increasing the number of " << hlp->id_name << " processes in your config file." ); */ -} - - static helper_request * Dequeue(helper * hlp) { @@ -1333,21 +1156,6 @@ Dequeue(helper * hlp) return r; } -static helper_stateful_request * -StatefulServerDequeue(helper_stateful_server * srv) -{ - dlink_node *link; - helper_stateful_request *r = NULL; - - if ((link = srv->queue.head)) { - r = (helper_stateful_request *)link->data; - dlinkDelete(link, &srv->queue); - memFree(link, MEM_DLINK_NODE); - } - - return r; -} - static helper_stateful_request * StatefulDequeue(statefulhelper * hlp) { @@ -1541,7 +1349,7 @@ helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r if (r->placeholder == 1) { /* a callback is needed before this request can _use_ a helper. */ - /* we don't care about releasing/deferring this helper. The request NEVER + /* we don't care about releasing this helper. The request NEVER * gets to the helper. So we throw away the return code */ r->callback(r->data, srv, NULL); /* throw away the placeholder */ @@ -1550,18 +1358,13 @@ helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r * request to the helper which is why we test for the request*/ if (srv->request == NULL) { - if (srv->flags.shutdown - && srv->flags.reserved == S_HELPER_FREE - && !srv->deferred_requests) { + if (srv->flags.shutdown && srv->flags.reserved == S_HELPER_FREE) { int wfd = srv->wfd; srv->wfd = -1; srv->flags.closing=1; comm_close(wfd); } else { - if (srv->queue.head) - helperStatefulServerKickQueue(srv); - else - helperStatefulKickQueue(hlp); + helperStatefulKickQueue(hlp); } } @@ -1605,15 +1408,6 @@ helperStatefulKickQueue(statefulhelper * hlp) helperStatefulDispatch(srv, r); } -static void -helperStatefulServerKickQueue(helper_stateful_server * srv) -{ - helper_stateful_request *r; - - if ((r = StatefulServerDequeue(srv))) - helperStatefulDispatch(srv, r); -} - static void helperRequestFree(helper_request * r) { diff --git a/src/helper.h b/src/helper.h index 1bb34a50fd..6939aa7729 100644 --- a/src/helper.h +++ b/src/helper.h @@ -152,7 +152,6 @@ struct _helper_stateful_server { struct timeval answer_time; dlink_node link; - dlink_list queue; statefulhelper *parent; helper_stateful_request *request; @@ -167,10 +166,7 @@ struct _helper_stateful_server { int uses; int submits; int releases; - int deferbyfunc; - int deferbycb; } stats; - int deferred_requests; /* current number of deferred requests */ void *data; /* State data used by the calling routines */ void *hIpc; }; @@ -196,7 +192,7 @@ public: MEMPROXY_CLASS(helper_stateful_request); char *buf; HLPSCB *callback; - int placeholder; /* if 1, this is a dummy request waiting for a stateful helper to become available for deferred requests.*/ + int placeholder; /* if 1, this is a dummy request waiting for a stateful helper to become available */ void *data; }; @@ -218,7 +214,6 @@ SQUIDCEXTERN void helperStatefulFree(statefulhelper *); SQUIDCEXTERN void helperStatefulReset(helper_stateful_server * srv); SQUIDCEXTERN void helperStatefulReleaseServer(helper_stateful_server * srv); SQUIDCEXTERN void *helperStatefulServerGetData(helper_stateful_server * srv); -SQUIDCEXTERN helper_stateful_server *helperStatefulDefer(statefulhelper *);