Polish: de-duplicate helper statistics objects

* Combine the stats structure on per-helper server classes. For more
  consistent statistic gathering.

* Add initStats() method to initialize statistics variables correctly.
  Previously only done for some counters on stateless helper objects.

* Add missing accounting of pending lookups in stateful helper code.

* Add counter for replies received from the helper.

* Add reporting of replies received back from each helper.

There are no logic or decision making logics affected by these changes.
The new increment/decrement and stats are purely affecting statistical
report outputs.

FUTURE TODO:
* replace the 'busy' flag on stateful helpers with pending>0 check
  as used by stateless helpers to indicate queue count.

diff --git a/src/helper.cc b/src/helper.cc
index 344c36dca2422cfd0374e73023e1e72bc67ab183..c5287607e831d3c3c96b0ed822741fa4b1262f6b 100644 (file)
--- a/src/helper.cc
+++ b/src/helper.cc
@@ -84,6 +84,15 @@ CBDATA_TYPE(helper_server);
 CBDATA_CLASS_INIT(statefulhelper);
 CBDATA_TYPE(helper_stateful_server);
 
+void
+HelperServerBase::initStats()
+{
+    stats.uses=0;
+    stats.replies=0;
+    stats.pending=0;
+    stats.releases=0;
+}
+
 void
 HelperServerBase::closePipesSafely()
 {
@@ -217,6 +226,7 @@ helperOpenServers(helper * hlp)
         srv = cbdataAlloc(helper_server);
         srv->hIpc = hIpc;
         srv->pid = pid;
+        srv->initStats();
         srv->index = k;
         srv->addr = hlp->addr;
         srv->readPipe = new Comm::Connection;
@@ -338,8 +348,7 @@ helperStatefulOpenServers(statefulhelper * hlp)
         srv->hIpc = hIpc;
         srv->pid = pid;
         srv->flags.reserved = 0;
-        srv->stats.submits = 0;
-        srv->stats.releases = 0;
+        srv->initStats();
         srv->index = k;
         srv->addr = hlp->addr;
         srv->readPipe = new Comm::Connection;
@@ -500,11 +509,12 @@ helperStats(StoreEntry * sentry, helper * 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%s\t%7s\t%7s\t%7s\n",
+    storeAppendPrintf(sentry, "%7s\t%7s\t%7s\t%11s\t%11s\t%s\t%7s\t%7s\t%7s\n",
                       "#",
                       "FD",
                       "PID",
                       "# Requests",
+                      "# Replies",
                       "Flags",
                       "Time",
                       "Offset",
@@ -513,11 +523,12 @@ helperStats(StoreEntry * sentry, helper * hlp, const char *label)
     for (dlink_node *link = hlp->servers.head; link; link = link->next) {
         helper_server *srv = (helper_server*)link->data;
         double tt = 0.001 * (srv->requests[0] ? tvSubMsec(srv->requests[0]->dispatch_time, current_time) : tvSubMsec(srv->dispatch_time, srv->answer_time));
-        storeAppendPrintf(sentry, "%7d\t%7d\t%7d\t%11d\t%c%c%c%c\t%7.3f\t%7d\t%s\n",
+        storeAppendPrintf(sentry, "%7d\t%7d\t%7d\t%11" PRIu64 "\t%11" PRIu64 "%c%c%c%c\t%7.3f\t%7d\t%s\n",
                           srv->index + 1,
                           srv->readPipe->fd,
                           srv->pid,
                           srv->stats.uses,
+                          srv->stats.replies,
                           srv->stats.pending ? 'B' : ' ',
                           srv->flags.writing ? 'W' : ' ',
                           srv->flags.closing ? 'C' : ' ',
@@ -553,11 +564,12 @@ 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%6s\t%7s\t%7s\t%7s\n",
+    storeAppendPrintf(sentry, "%7s\t%7s\t%7s\t%11s\t%11s\t%6s\t%7s\t%7s\t%7s\n",
                       "#",
                       "FD",
                       "PID",
                       "# Requests",
+                      "# Replies",
                       "Flags",
                       "Time",
                       "Offset",
@@ -566,11 +578,12 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label
     for (dlink_node *link = hlp->servers.head; link; link = link->next) {
         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%c%c%c%c%c\t%7.3f\t%7d\t%s\n",
+        storeAppendPrintf(sentry, "%7d\t%7d\t%7d\t%11" PRIu64 "\t%11" PRIu64 "\t%c%c%c%c%c\t%7.3f\t%7d\t%s\n",
                           srv->index + 1,
                           srv->readPipe->fd,
                           srv->pid,
                           srv->stats.uses,
+                          srv->stats.replies,
                           srv->flags.busy ? 'B' : ' ',
                           srv->flags.closing ? 'C' : ' ',
                           srv->flags.reserved ? 'R' : ' ',
@@ -829,6 +842,7 @@ static void helperReturnBuffer(int request_number, helper_server * srv, helper *
             callback(cbdata, msg);
 
         -- srv->stats.pending;
+        ++ srv->stats.replies;
 
         ++ hlp->stats.replies;
 
@@ -1005,6 +1019,10 @@ helperStatefulHandleRead(const Comm::ConnectionPointer &conn, char *buf, size_t
         srv->roffset = 0;
         helperStatefulRequestFree(r);
         srv->request = NULL;
+
+        -- srv->stats.pending;
+        ++ srv->stats.replies;
+
         ++ hlp->stats.replies;
         srv->answer_time = current_time;
         hlp->stats.avg_svc_time =
@@ -1272,7 +1290,6 @@ helperDispatch(helper_server * srv, helper_request * r)
 
     assert(ptr);
     *ptr = r;
-    srv->stats.pending += 1;
     r->dispatch_time = current_time;
 
     if (srv->wqueue->isNull())
@@ -1296,6 +1313,7 @@ helperDispatch(helper_server * srv, helper_request * r)
     debugs(84, 5, "helperDispatch: Request sent to " << hlp->id_name << " #" << srv->index + 1 << ", " << strlen(r->buf) << " bytes");
 
     ++ srv->stats.uses;
+    ++ srv->stats.pending;
     ++ hlp->stats.requests;
 }
 
@@ -1348,6 +1366,7 @@ helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r
            (int) strlen(r->buf) << " bytes");
 
     ++ srv->stats.uses;
+    ++ srv->stats.pending;
     ++ hlp->stats.requests;
 }
 

diff --git a/src/helper.h b/src/helper.h
index 5da5c3b752e549694dbf73f7bce27a631d582b93..21bd17730d177eeb2b2d3258ff4d460e60822331 100644 (file)
--- a/src/helper.h
+++ b/src/helper.h
@@ -88,7 +88,7 @@ private:
     CBDATA_CLASS2(statefulhelper);
 };
 
-/*
+/**
  * Fields shared between stateless and stateful helper servers.
  */
 class HelperServerBase
@@ -130,6 +130,13 @@ public:
         unsigned int reserved:1;
     } flags;
 
+    struct {
+        uint64_t uses;     //< requests sent to this helper
+        uint64_t replies;  //< replies received from this helper
+        uint64_t pending;  //< queued lookups waiting to be sent to this helper
+        uint64_t releases; //< times release() has been called on this helper (if stateful)
+    } stats;
+    void initStats();
 };
 
 class MemBuf;
@@ -143,11 +150,6 @@ public:
     helper *parent;
     helper_request **requests;
 
-    struct {
-        int uses;
-        unsigned int pending;
-    } stats;
-
 private:
     CBDATA_CLASS2(helper_server);
 };
@@ -163,11 +165,6 @@ public:
     statefulhelper *parent;
     helper_stateful_request *request;
 
-    struct {
-        int uses;
-        int submits;
-        int releases;
-    } stats;
     void *data;                        /* State data used by the calling routines */
 
 private: