[PATCH 8/9] Do not start useless diskers.

From: Dmitry Kurochkin <dmitry.kurochkin_at_measurement-factory.com>
Date: Thu, 29 Sep 2011 03:06:06 +0400

Before the change, a disker process was started for each configured
cache_dir. But not all cache_dirs need a disker: only Rock store
requires one when running in SMP mode. There was some existing code
to avoid starting useless diskers (see SwapDir::needsDiskStrand and
Config.cacheSwap.n_strands) but it was not complete. The patch fixes
this issue.

---
 src/DiskIO/IpcIo/IpcIoFile.cc |    2 +-
 src/Makefile.am               |    1 +
 src/SwapDir.cc                |    4 ++--
 src/SwapDir.h                 |    1 +
 src/cache_cf.cc               |   10 +++++++---
 src/fs/rock/RockSwapDir.cc    |    7 +++++--
 src/ipc/Kids.cc               |    3 +--
 7 files changed, 18 insertions(+), 10 deletions(-)
diff --git src/DiskIO/IpcIo/IpcIoFile.cc src/DiskIO/IpcIo/IpcIoFile.cc
index 13c0d00..3c7cc7e 100644
--- src/DiskIO/IpcIo/IpcIoFile.cc
+++ src/DiskIO/IpcIo/IpcIoFile.cc
@@ -792,7 +792,7 @@ void IpcIoRr::create(const RunnerRegistry &)
     Must(!owner);
     // XXX: make capacity configurable
     owner = Ipc::FewToFewBiQueue::Init(ShmLabel, Config.workers, 1,
-                                       Config.cacheSwap.n_configured,
+                                       Config.cacheSwap.n_strands,
                                        1 + Config.workers, sizeof(IpcIoMsg),
                                        1024);
 }
diff --git src/Makefile.am src/Makefile.am
index a794717..8d80a0e 100644
--- src/Makefile.am
+++ src/Makefile.am
@@ -2146,6 +2146,7 @@ tests_testHttpRequest_SOURCES = \
 	tests/testHttpRequestMethod.h \
 	tests/testHttpRequestMethod.cc \
 	tests/testMain.cc \
+	tests/stub_DiskIOModule.cc \
 	tests/stub_main_cc.cc \
 	tests/stub_ipc_Forwarder.cc \
 	time.cc \
diff --git src/SwapDir.cc src/SwapDir.cc
index 12ee586..6eef047 100644
--- src/SwapDir.cc
+++ src/SwapDir.cc
@@ -40,7 +40,7 @@
 
 SwapDir::SwapDir(char const *aType): theType(aType),
         max_size(0),
-        path(NULL), index(-1), min_objsize(0), max_objsize (-1),
+        path(NULL), index(-1), disker(-1), min_objsize(0), max_objsize (-1),
         repl(NULL), removals(0), scanned(0),
         cleanLog(NULL)
 {
@@ -198,7 +198,7 @@ SwapDir::active() const
         return true;
 
     // we are inside a disker dedicated to this disk
-    if (IamDiskProcess() && index == (KidIdentifier-1 - Config.workers))
+    if (KidIdentifier == disker)
         return true;
 
     return false; // Coordinator, wrong disker, etc.
diff --git src/SwapDir.h src/SwapDir.h
index f43b05c..723443c 100644
--- src/SwapDir.h
+++ src/SwapDir.h
@@ -179,6 +179,7 @@ protected:
 public:
     char *path;
     int index;			/* This entry's index into the swapDirs array */
+    int disker; ///< disker kid id dedicated to this SwapDir or -1
     int64_t min_objsize;
     int64_t max_objsize;
     RemovalPolicy *repl;
diff --git src/cache_cf.cc src/cache_cf.cc
index ad9a74d..88f1597 100644
--- src/cache_cf.cc
+++ src/cache_cf.cc
@@ -56,6 +56,7 @@
 #endif
 #include "ConfigParser.h"
 #include "CpuAffinityMap.h"
+#include "DiskIO/DiskIOModule.h"
 #include "eui/Config.h"
 #if USE_SQUID_ESI
 #include "esi/Parser.h"
@@ -599,6 +600,12 @@ configDoConfigure(void)
         /* Memory-only cache probably in effect. */
         /* turn off the cache rebuild delays... */
         StoreController::store_dirs_rebuilding = 0;
+    } else if (InDaemonMode()) { // no diskers in non-daemon mode
+        for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
+            const RefCount<SwapDir> sd = Config.cacheSwap.swapDirs[i];
+            if (sd->needsDiskStrand())
+                sd->disker = Config.workers + ++Config.cacheSwap.n_strands;
+        }
     }
 
     if (Debug::rotateNumber < 0) {
@@ -1987,9 +1994,6 @@ parse_cachedir(SquidConfig::_cacheSwap * swap)
 
     ++swap->n_configured;
 
-    if (sd->needsDiskStrand())
-        ++swap->n_strands;
-
     /* Update the max object size */
     update_maxobjsize();
 }
diff --git src/fs/rock/RockSwapDir.cc src/fs/rock/RockSwapDir.cc
index d500ce4..4aefc21 100644
--- src/fs/rock/RockSwapDir.cc
+++ src/fs/rock/RockSwapDir.cc
@@ -215,7 +215,7 @@ Rock::SwapDir::init()
     Must(!map);
     map = new DirMap(path);
 
-    const char *ioModule = UsingSmp() ? "IpcIo" : "Blocking";
+    const char *ioModule = needsDiskStrand() ? "IpcIo" : "Blocking";
     if (DiskIOModule *m = DiskIOModule::Find(ioModule)) {
         debugs(47,2, HERE << "Using DiskIO module: " << ioModule);
         io = m->createStrategy();
@@ -239,7 +239,10 @@ Rock::SwapDir::init()
 bool
 Rock::SwapDir::needsDiskStrand() const
 {
-    return true;
+    const bool wontEvenWorkWithoutDisker = Config.workers > 1;
+    const bool wouldWorkBetterWithDisker = DiskIOModule::Find("IpcIo");
+    return InDaemonMode() && (wontEvenWorkWithoutDisker ||
+        wouldWorkBetterWithDisker);
 }
 
 void
diff --git src/ipc/Kids.cc src/ipc/Kids.cc
index 55f877f..7d36315 100644
--- src/ipc/Kids.cc
+++ src/ipc/Kids.cc
@@ -33,8 +33,7 @@ void Kids::init()
     }
 
     // add Kid records for all disk processes
-    // (XXX: some cache_dirs do not need this)
-    for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
+    for (int i = 0; i < Config.cacheSwap.n_strands; ++i) {
         snprintf(kid_name, sizeof(kid_name), "(squid-disk-%d)", (int)(storage.size()+1));
         storage.push_back(Kid(kid_name));
     }
-- 
1.7.6.3
Received on Wed Sep 28 2011 - 23:07:35 MDT

This archive was generated by hypermail 2.2.0 : Thu Sep 29 2011 - 12:00:03 MDT