Do not start useless diskers.

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.

diff --git a/src/DiskIO/IpcIo/IpcIoFile.cc b/src/DiskIO/IpcIo/IpcIoFile.cc
index a00a8035cecdbc26c119b217b2404a04ee7046d3..aec7b40700899447e09fe1278767c142ca25f8f5 100644 (file)
--- a/src/DiskIO/IpcIo/IpcIoFile.cc
+++ b/src/DiskIO/IpcIo/IpcIoFile.cc
@@ -856,7 +856,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 a/src/Makefile.am b/src/Makefile.am
index a794717266e1b6a6730a6b4fec526d6aaa37c7a2..8d80a0e8fd5f0661bd1ab79926bbea3d91a11a3c 100644 (file)
--- a/src/Makefile.am
+++ b/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 a/src/SwapDir.cc b/src/SwapDir.cc
index 12ee586b573831d559b4515f22a0e59bca1fbc60..6eef047089a6b9d11d69eea25bd3196e95453726 100644 (file)
--- a/src/SwapDir.cc
+++ b/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 a/src/SwapDir.h b/src/SwapDir.h
index f43b05c0e99ecc8926d4b35372d292527f185751..723443cfadacfa76fb127a231f8f1ac4d60cc098 100644 (file)
--- a/src/SwapDir.h
+++ b/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 a/src/cache_cf.cc b/src/cache_cf.cc
index ad9a74d9ee5a5fa07bd22f69d7ae4c39edd6a2eb..24079dd32c3fee2db0285de0163a42f676199c8b 100644 (file)
--- a/src/cache_cf.cc
+++ b/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 a/src/fs/rock/RockSwapDir.cc b/src/fs/rock/RockSwapDir.cc
index 2c18b0d0428ba522e1b0e62840999215f33fde40..7073cd8c5989ed45803a0d0571bbcd315cbf71db 100644 (file)
--- a/src/fs/rock/RockSwapDir.cc
+++ b/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 a/src/ipc/Kids.cc b/src/ipc/Kids.cc
index 55f877f890493e8ad235a02a377cc14112664027..7d363151aa558eeb62accab8be650593c46ddc39 100644 (file)
--- a/src/ipc/Kids.cc
+++ b/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));
     }