Bug 4579: cannot hit an entry being written by another worker (#183)

... and addressed XXX in 4310f8b commit.

This change disassociates Transients from collapsed forwarding, enabling
it for SMP caching configurations. Before this change, SMP Squid worker
could not read an entry being written by another worker. Besides
unexpected misses, there could be another (worse) negative effect: The
reader worker could get stuck because it did not get updates via
the Transients mechanism.

Also deprecate the collapsed_forwarding_shared_entries_limit directive
name in favor of shared_transient_entries_limit.

Also removed top-level Storage::smpAware() because memory cache SMP
awareness is determined by configuration and is now computed before we
create the memory cache Storage object. This ability to assess SMP
awareness earlier helps decide whether to create Transients segments.

Also eliminated code duplication in a couple of MemStoreRr methods.

diff --git a/src/Makefile.am b/src/Makefile.am
index 9511f470bc4fde4ec563e18850467ed96954c0ba..dc11f8865714005b27e97dce3b9eb980018cdc20 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2719,6 +2719,7 @@ tests_testStore_SOURCES= \
        mem_node.cc \
        MemBuf.cc \
        MemObject.cc \
+       MemStore.cc \
        Notes.h \
        Notes.cc \
        Parsing.cc \
@@ -2772,7 +2773,6 @@ tests_testStore_SOURCES= \
        tests/stub_HttpReply.cc \
        tests/stub_HttpRequest.cc \
        tests/stub_libcomm.cc \
-       tests/stub_MemStore.cc \
        mime.h \
        tests/stub_mime.cc \
        tests/stub_Port.cc \
@@ -2907,7 +2907,6 @@ tests_testUfs_SOURCES = \
        tests/stub_libeui.cc \
        tests/stub_libicmp.cc \
        tests/stub_liblog.cc \
-       tests/stub_MemStore.cc \
        tests/stub_neighbors.cc \
        tests/stub_pconn.cc \
        tests/stub_Port.cc \
@@ -2944,6 +2943,7 @@ tests_testUfs_SOURCES = \
        MasterXaction.cc \
        MasterXaction.h \
        MemObject.cc \
+       MemStore.cc \
        Notes.h \
        Notes.cc \
        StoreSwapLogData.cc \
@@ -3127,6 +3127,7 @@ tests_testRock_SOURCES = \
        MasterXaction.h \
        MemBuf.cc \
        MemObject.cc \
+       MemStore.cc \
        mem_node.cc \
        Notes.h \
        Notes.cc \
@@ -3185,7 +3186,6 @@ tests_testRock_SOURCES = \
        tests/stub_liblog.cc \
        tests/stub_libmgr.cc \
        tests/stub_libsecurity.cc \
-       tests/stub_MemStore.cc \
        mime.h \
        tests/stub_mime.cc \
        tests/stub_neighbors.cc \

diff --git a/src/MemStore.cc b/src/MemStore.cc
index 732451a77cd41550032bb02570ec3dde7475ef12..fd1ad65fa2dbba755d8b0df810ed2feb5fff15f7 100644 (file)
--- a/src/MemStore.cc
+++ b/src/MemStore.cc
@@ -177,7 +177,7 @@ MemStore::init()
 {
     const int64_t entryLimit = EntryLimit();
     if (entryLimit <= 0)
-        return; // no memory cache configured or a misconfiguration
+        return; // no shared memory cache configured or a misconfiguration
 
     // check compatibility with the disk cache, if any
     if (Config.cacheSwap.n_configured > 0) {
@@ -927,12 +927,18 @@ MemStore::disconnect(StoreEntry &e)
     }
 }
 
+bool
+MemStore::Requested()
+{
+    return Config.memShared && Config.memMaxSize > 0;
+}
+
 /// calculates maximum number of entries we need to store and map
 int64_t
 MemStore::EntryLimit()
 {
-    if (!Config.memShared || !Config.memMaxSize)
-        return 0; // no memory cache configured
+    if (!Requested())
+        return 0;
 
     const int64_t minEntrySize = Ipc::Mem::PageSize();
     const int64_t entryLimit = Config.memMaxSize / minEntrySize;
@@ -983,6 +989,12 @@ MemStoreRr::finalizeConfig()
         debugs(20, DBG_IMPORTANT, "WARNING: memory_cache_shared is on, but only"
                " a single worker is running");
     }
+
+    if (MemStore::Requested() && Config.memMaxSize < Ipc::Mem::PageSize()) {
+        debugs(20, DBG_IMPORTANT, "WARNING: mem-cache size is too small (" <<
+               (Config.memMaxSize / 1024.0) << " KB), should be >= " <<
+               (Ipc::Mem::PageSize() / 1024.0) << " KB");
+    }
 }
 
 void
@@ -995,19 +1007,11 @@ MemStoreRr::useConfig()
 void
 MemStoreRr::create()
 {
-    if (!Config.memShared)
+    if (!MemStore::Enabled())
         return;
 
     const int64_t entryLimit = MemStore::EntryLimit();
-    if (entryLimit <= 0) {
-        if (Config.memMaxSize > 0) {
-            debugs(20, DBG_IMPORTANT, "WARNING: mem-cache size is too small ("
-                   << (Config.memMaxSize / 1024.0) << " KB), should be >= " <<
-                   (Ipc::Mem::PageSize() / 1024.0) << " KB");
-        }
-        return; // no memory cache configured or a misconfiguration
-    }
-
+    assert(entryLimit > 0);
     Must(!spaceOwner);
     spaceOwner = shm_new(Ipc::Mem::PageStack)(SpaceLabel, SpacePoolId,
                  entryLimit, 0);

diff --git a/src/MemStore.h b/src/MemStore.h
index 8d771e33a7dd0a7e78819d25ea658de617eda56d..7233046da24d2edfff8c34867f9f12ff14351d0b 100644 (file)
--- a/src/MemStore.h
+++ b/src/MemStore.h
@@ -63,9 +63,13 @@ public:
     virtual bool updateAnchored(StoreEntry &) override;
     virtual void evictCached(StoreEntry &) override;
     virtual void evictIfFound(const cache_key *) override;
-    virtual bool smpAware() const override { return true; }
 
+    /// whether Squid is correctly configured to use a shared memory cache
+    static bool Enabled() { return EntryLimit() > 0; }
     static int64_t EntryLimit();
+    /// whether Squid is configured to use a shared memory cache
+    /// (it may still be disabled due to the implicit minimum entry size limit)
+    static bool Requested();
 
 protected:
     friend ShmWriter;

diff --git a/src/SquidConfig.h b/src/SquidConfig.h
index 19233f47a7c458baec9206cd779bfde720e45875..adbfd61e13c020e4b8c07b8fc92da77c98cc2dd9 100644 (file)
--- a/src/SquidConfig.h
+++ b/src/SquidConfig.h
@@ -342,7 +342,7 @@ public:
 #endif
     } onoff;
 
-    int64_t collapsed_forwarding_shared_entries_limit;
+    int64_t shared_transient_entries_limit;
 
     int pipeline_max_prefetch;
 

diff --git a/src/Transients.cc b/src/Transients.cc
index f79710ad79067555ac29f967f1cb1adbe121687d..460b0a69ec38d2c7e705d566e68f201a83a37b6c 100644 (file)
--- a/src/Transients.cc
+++ b/src/Transients.cc
@@ -40,9 +40,9 @@ Transients::~Transients()
 void
 Transients::init()
 {
+    assert(Enabled());
     const int64_t entryLimit = EntryLimit();
-    if (entryLimit <= 0)
-        return; // no SMP support or a misconfiguration
+    assert(entryLimit > 0);
 
     Must(!map);
     map = new TransientsMap(MapLabel);
@@ -337,11 +337,8 @@ Transients::disconnect(StoreEntry &entry)
 int64_t
 Transients::EntryLimit()
 {
-    // TODO: we should also check whether any SMP-aware caching is configured
-    if (!UsingSmp() || !Config.onoff.collapsed_forwarding)
-        return 0; // no SMP collapsed forwarding possible or needed
-
-    return Config.collapsed_forwarding_shared_entries_limit;
+    return (UsingSmp() && Store::Controller::SmpAware()) ?
+           Config.shared_transient_entries_limit : 0;
 }
 
 bool
@@ -390,9 +387,6 @@ TransientsRr::useConfig()
 void
 TransientsRr::create()
 {
-    if (!Config.onoff.collapsed_forwarding)
-        return;
-
     const int64_t entryLimit = Transients::EntryLimit();
     if (entryLimit <= 0)
         return; // no SMP configured or a misconfiguration

diff --git a/src/Transients.h b/src/Transients.h
index 7cc181157915272bfabeb8b81f65c0d568245a22..ce27ad374bacdeb05e4adef08b69ff71ab9b90b7 100644 (file)
--- a/src/Transients.h
+++ b/src/Transients.h
@@ -76,7 +76,6 @@ public:
     virtual void evictCached(StoreEntry &) override;
     virtual void evictIfFound(const cache_key *) override;
     virtual void maintain() override;
-    virtual bool smpAware() const override { return true; }
 
     /// Whether an entry with the given public key exists and (but) was
     /// marked for removal some time ago; get(key) returns nil in such cases.
@@ -89,6 +88,9 @@ public:
 
     static int64_t EntryLimit();
 
+    /// Can we create and initialize Transients?
+    static bool Enabled() { return EntryLimit(); }
+
 protected:
     void addEntry(StoreEntry*, const cache_key *, const Store::IoStatus);
 

diff --git a/src/cf.data.pre b/src/cf.data.pre
index da82f835b8ace6746d2e9c06ce3631e89c75e217..23dbedcbd0329aa79260b47e6fff236a2eb89205 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -6665,25 +6665,36 @@ DOC_START
        See also: collapsed_forwarding.
 DOC_END
 
-NAME: collapsed_forwarding_shared_entries_limit
+NAME: shared_transient_entries_limit collapsed_forwarding_shared_entries_limit
 COMMENT: (number of entries)
 TYPE: int64_t
-LOC: Config.collapsed_forwarding_shared_entries_limit
+LOC: Config.shared_transient_entries_limit
 DEFAULT: 16384
 DOC_START
-       This limits the size of a table used for sharing information
-       about collapsible entries among SMP workers. Limiting sharing
-       too much results in cache content duplication and missed
-       collapsing opportunities. Using excessively large values
-       wastes shared memory.
+       This directive limits the size of a table used for sharing current
+       transaction information among SMP workers. A table entry stores meta
+       information about a single cache entry being delivered to Squid
+       client(s) by one or more SMP workers. A single table entry consumes
+       less than 128 shared memory bytes.
 
-       The limit should be significantly larger then the number of
-       concurrent collapsible entries one wants to share. For a cache
-       that handles less than 5000 concurrent requests, the default
+       The limit should be significantly larger than the number of
+       concurrent non-collapsed cachable responses leaving Squid. For a
+       cache that handles less than 5000 concurrent requests, the default
        setting of 16384 should be plenty.
 
-       If the limit is set to zero, it disables sharing of collapsed
-       forwarding between SMP workers.
+       Using excessively large values wastes shared memory. Limiting the
+       table size too much results in hash collisions, leading to lower hit
+       ratio and missed SMP request collapsing opportunities: Transactions
+       left without a table entry cannot cache their responses and are
+       invisible to other concurrent requests for the same resource.
+
+       A zero limit is allowed but unsupported. A positive small limit
+       lowers hit ratio, but zero limit disables a lot of essential
+       synchronization among SMP workers, leading to HTTP violations (e.g.,
+       stale hit responses). It also disables shared collapsed forwarding:
+       A worker becomes unable to collapse its requests on transactions in
+       other workers, resulting in more trips to the origin server and more
+       cache thrashing.
 DOC_END
 
 COMMENT_START

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 821dfaad39289386289b4582dcfa89a0c291907b..fc0e667f6d580812763b8d2ca08acc768b030c20 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -298,7 +298,7 @@ clientReplyContext::processExpired()
     // TODO: Consider also allowing regular (non-collapsed) revalidation hits.
     // TODO: support collapsed revalidation for Vary-controlled entries
     bool collapsingAllowed = Config.onoff.collapsed_forwarding &&
-                             !Store::Root().smpAware() &&
+                             !Store::Controller::SmpAware() &&
                              http->request->vary_headers.isEmpty();
 
     StoreEntry *entry = nullptr;

diff --git a/src/store.cc b/src/store.cc
index 30b0307db83e599e21379b9d861479a1093f28c8..6d5e355451951e917ef208745eae95bf59c4dd25 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -23,6 +23,7 @@
 #include "HttpRequest.h"
 #include "mem_node.h"
 #include "MemObject.h"
+#include "MemStore.h"
 #include "mgr/Registration.h"
 #include "mgr/StoreIoAction.h"
 #include "profiler/Profiler.h"
@@ -1578,7 +1579,7 @@ StoreEntry::setMemStatus(mem_status_t new_status)
         return;
 
     // are we using a shared memory cache?
-    if (Config.memShared && IamWorkerProcess()) {
+    if (MemStore::Enabled()) {
         // This method was designed to update replacement policy, not to
         // actually purge something from the memory cache (TODO: rename?).
         // Shared memory cache does not have a policy that needs updates.

diff --git a/src/store/Controller.cc b/src/store/Controller.cc
index a822b39f3dd945a695de7f7bf66692b711ae2dcb..3ed7b5ad402937a0127216a220a7356b2d10d0d8 100644 (file)
--- a/src/store/Controller.cc
+++ b/src/store/Controller.cc
@@ -35,7 +35,8 @@ int Store::Controller::store_dirs_rebuilding = 1;
 
 Store::Controller::Controller() :
     swapDir(new Disks),
-    memStore(NULL),
+    sharedMemStore(nullptr),
+    localMemStore(false),
     transients(NULL)
 {
     assert(!store_table);
@@ -43,7 +44,7 @@ Store::Controller::Controller() :
 
 Store::Controller::~Controller()
 {
-    delete memStore;
+    delete sharedMemStore;
     delete transients;
     delete swapDir;
 
@@ -57,15 +58,18 @@ Store::Controller::~Controller()
 void
 Store::Controller::init()
 {
-    if (Config.memShared && IamWorkerProcess()) {
-        memStore = new MemStore;
-        memStore->init();
+    if (IamWorkerProcess()) {
+        if (MemStore::Enabled()) {
+            sharedMemStore = new MemStore;
+            sharedMemStore->init();
+        } else if (Config.memMaxSize > 0) {
+            localMemStore = true;
+        }
     }
 
     swapDir->init();
 
-    if (UsingSmp() && IamWorkerProcess() && Config.onoff.collapsed_forwarding &&
-            smpAware()) {
+    if (Transients::Enabled() && IamWorkerProcess()) {
         transients = new Transients;
         transients->init();
     }
@@ -110,14 +114,20 @@ Store::Controller::maintain()
 void
 Store::Controller::getStats(StoreInfoStats &stats) const
 {
-    if (memStore)
-        memStore->getStats(stats);
+    if (sharedMemStore)
+        sharedMemStore->getStats(stats);
     else {
         // move this code to a non-shared memory cache class when we have it
         stats.mem.shared = false;
         stats.mem.capacity = Config.memMaxSize;
         stats.mem.size = mem_node::StoreMemSize();
-        stats.mem.count = hot_obj_count;
+        if (localMemStore) {
+            // XXX: also count internal/in-transit objects
+            stats.mem.count = hot_obj_count;
+        } else {
+            // XXX: count internal/in-transit objects instead
+            stats.mem.count = hot_obj_count;
+        }
     }
 
     swapDir->getStats(stats);
@@ -141,8 +151,8 @@ Store::Controller::stat(StoreEntry &output) const
                       Math::doublePercent(currentSize(), maxSize()),
                       Math::doublePercent((maxSize() - currentSize()), maxSize()));
 
-    if (memStore)
-        memStore->stat(output);
+    if (sharedMemStore)
+        sharedMemStore->stat(output);
 
     /* now the swapDir */
     swapDir->stat(output);
@@ -211,8 +221,8 @@ Store::Controller::search()
 void
 Store::Controller::sync(void)
 {
-    if (memStore)
-        memStore->sync();
+    if (sharedMemStore)
+        sharedMemStore->sync();
     swapDir->sync();
 }
 
@@ -247,8 +257,8 @@ Store::Controller::referenceBusy(StoreEntry &e)
         swapDir->reference(e);
 
     // Notify the memory cache that we're referencing this object again
-    if (memStore && e.mem_status == IN_MEMORY)
-        memStore->reference(e);
+    if (sharedMemStore && e.mem_status == IN_MEMORY)
+        sharedMemStore->reference(e);
 
     // TODO: move this code to a non-shared memory cache class when we have it
     if (e.mem_obj) {
@@ -274,15 +284,15 @@ Store::Controller::dereferenceIdle(StoreEntry &e, bool wantsLocalMemory)
         keepInStoreTable = swapDir->dereference(e) || keepInStoreTable;
 
     // Notify the memory cache that we're not referencing this object any more
-    if (memStore && e.mem_status == IN_MEMORY)
-        keepInStoreTable = memStore->dereference(e) || keepInStoreTable;
+    if (sharedMemStore && e.mem_status == IN_MEMORY)
+        keepInStoreTable = sharedMemStore->dereference(e) || keepInStoreTable;
 
     // TODO: move this code to a non-shared memory cache class when we have it
     if (e.mem_obj) {
         if (mem_policy->Dereferenced)
             mem_policy->Dereferenced(mem_policy, &e, &e.mem_obj->repl);
         // non-shared memory cache relies on store_table
-        if (!memStore)
+        if (localMemStore)
             keepInStoreTable = wantsLocalMemory || keepInStoreTable;
     }
 
@@ -403,8 +413,8 @@ Store::Controller::peek(const cache_key *key)
         }
     }
 
-    if (memStore) {
-        if (StoreEntry *e = memStore->get(key)) {
+    if (sharedMemStore) {
+        if (StoreEntry *e = sharedMemStore->get(key)) {
             debugs(20, 3, HERE << "got mem-cached entry: " << *e);
             return e;
         }
@@ -465,8 +475,8 @@ Store::Controller::evictIfFound(const cache_key *key)
         return;
     }
 
-    if (memStore)
-        memStore->evictIfFound(key);
+    if (sharedMemStore)
+        sharedMemStore->evictIfFound(key);
     if (swapDir)
         swapDir->evictIfFound(key);
     if (transients)
@@ -546,9 +556,9 @@ void
 Store::Controller::memoryOut(StoreEntry &e, const bool preserveSwappable)
 {
     bool keepInLocalMemory = false;
-    if (memStore)
-        memStore->write(e); // leave keepInLocalMemory false
-    else
+    if (sharedMemStore)
+        sharedMemStore->write(e); // leave keepInLocalMemory false
+    else if (localMemStore)
         keepInLocalMemory = keepForLocalMemoryCache(e);
 
     debugs(20, 7, HERE << "keepInLocalMemory: " << keepInLocalMemory);
@@ -563,8 +573,8 @@ void
 Store::Controller::memoryEvictCached(StoreEntry &e)
 {
     // TODO: Untangle memory caching from mem_obj.
-    if (memStore)
-        memStore->evictCached(e);
+    if (sharedMemStore)
+        sharedMemStore->evictCached(e);
     else // TODO: move into [non-shared] memory cache class when we have one
         if (!e.locked())
             e.destroyMemObject();
@@ -573,8 +583,8 @@ Store::Controller::memoryEvictCached(StoreEntry &e)
 void
 Store::Controller::memoryDisconnect(StoreEntry &e)
 {
-    if (memStore)
-        memStore->disconnect(e);
+    if (sharedMemStore)
+        sharedMemStore->disconnect(e);
     // else nothing to do for non-shared memory cache
 }
 
@@ -630,9 +640,9 @@ Store::Controller::handleIdleEntry(StoreEntry &e)
         // have a dedicated storage for them (that would not purge them).
         // They are not managed [well] by any specific Store handled below.
         keepInLocalMemory = true;
-    } else if (memStore) {
-        // leave keepInLocalMemory false; memStore maintains its own cache
-    } else {
+    } else if (sharedMemStore) {
+        // leave keepInLocalMemory false; sharedMemStore maintains its own cache
+    } else if (localMemStore) {
         keepInLocalMemory = keepForLocalMemoryCache(e) && // in good shape and
                             // the local memory cache is not overflowing
                             memoryCacheHasSpaceFor(memoryPagesDebt_);
@@ -680,8 +690,8 @@ Store::Controller::updateOnNotModified(StoreEntry *old, const StoreEntry &newer)
 
     /* update stored image of the old entry */
 
-    if (memStore && old->mem_status == IN_MEMORY && !EBIT_TEST(old->flags, ENTRY_SPECIAL))
-        memStore->updateHeaders(old);
+    if (sharedMemStore && old->mem_status == IN_MEMORY && !EBIT_TEST(old->flags, ENTRY_SPECIAL))
+        sharedMemStore->updateHeaders(old);
 
     if (old->hasDisk())
         swapDir->updateHeaders(old);
@@ -782,13 +792,13 @@ Store::Controller::syncCollapsed(const sfileno xitIndex)
 
     bool found = false;
     bool inSync = false;
-    if (memStore && collapsed->mem_obj->memCache.io == MemObject::ioDone) {
+    if (sharedMemStore && collapsed->mem_obj->memCache.io == MemObject::ioDone) {
         found = true;
         inSync = true;
         debugs(20, 7, "fully mem-loaded " << *collapsed);
-    } else if (memStore && collapsed->hasMemStore()) {
+    } else if (sharedMemStore && collapsed->hasMemStore()) {
         found = true;
-        inSync = memStore->updateAnchored(*collapsed);
+        inSync = sharedMemStore->updateAnchored(*collapsed);
         // TODO: handle entries attached to both memory and disk
     } else if (swapDir && collapsed->hasDisk()) {
         found = true;
@@ -832,8 +842,8 @@ Store::Controller::anchorToCache(StoreEntry &entry, bool &inSync)
     debugs(20, 7, "anchoring " << entry);
 
     bool found = false;
-    if (memStore)
-        found = memStore->anchorToCache(entry, inSync);
+    if (sharedMemStore)
+        found = sharedMemStore->anchorToCache(entry, inSync);
     if (!found && swapDir)
         found = swapDir->anchorToCache(entry, inSync);
 
@@ -850,9 +860,9 @@ Store::Controller::anchorToCache(StoreEntry &entry, bool &inSync)
 }
 
 bool
-Store::Controller::smpAware() const
+Store::Controller::SmpAware()
 {
-    return memStore || (swapDir && swapDir->smpAware());
+    return MemStore::Enabled() || Disks::SmpAware();
 }
 
 void

diff --git a/src/store/Controller.h b/src/store/Controller.h
index cdd88e13c37433b0a8373845d7c39d87532ab3dc..b73c5d2b93024f281cf7f1ccf91e4a6fa603f601 100644 (file)
--- a/src/store/Controller.h
+++ b/src/store/Controller.h
@@ -40,7 +40,6 @@ public:
     virtual void evictCached(StoreEntry &) override;
     virtual void evictIfFound(const cache_key *) override;
     virtual int callback() override;
-    virtual bool smpAware() const override;
 
     /// \returns a locally indexed and SMP-tracked matching StoreEntry (or nil)
     /// Slower than peek() but does not restrict StoreEntry use and storage.
@@ -133,6 +132,9 @@ public:
     /// \returns an iterator for all Store entries
     StoreSearch *search();
 
+    /// whether there are any SMP-aware storages
+    static bool SmpAware();
+
     /// the number of cache_dirs being rebuilt; TODO: move to Disks::Rebuilding
     static int store_dirs_rebuilding;
 
@@ -152,7 +154,8 @@ private:
     void checkTransients(const StoreEntry &) const;
 
     Disks *swapDir; ///< summary view of all disk caches
-    Memory *memStore; ///< memory cache
+    Memory *sharedMemStore; ///< memory cache that multiple workers can use
+    bool localMemStore; ///< whether local (non-shared) memory cache is enabled
 
     /// A shared table of public store entries that do not know whether they
     /// will belong to a memory cache, a disk cache, or will be uncachable

diff --git a/src/store/Disk.h b/src/store/Disk.h
index 416839b387799e69c6adee718a0099ebdeca3af5..0feae51677feaf4bc603c6ebfd65d267fb04d67d 100644 (file)
--- a/src/store/Disk.h
+++ b/src/store/Disk.h
@@ -53,7 +53,8 @@ public:
     virtual void reference(StoreEntry &e) override;
     virtual bool dereference(StoreEntry &e) override;
     virtual void maintain() override;
-    virtual bool smpAware() const override { return false; }
+    /// whether this disk storage is capable of serving multiple workers
+    virtual bool smpAware() const = 0;
 
     /// the size of the smallest entry this cache_dir can store
     int64_t minObjectSize() const;

diff --git a/src/store/Disks.cc b/src/store/Disks.cc
index b543d84402ad9c1c651b3581943b9c1a6faed578..3a95a4a96c9bda1fdb6b84b81ee5e7cbca7b5b5a 100644 (file)
--- a/src/store/Disks.cc
+++ b/src/store/Disks.cc
@@ -162,7 +162,7 @@ Store::Disks::store(int const x) const
 }
 
 SwapDir &
-Store::Disks::dir(const int i) const
+Store::Disks::Dir(const int i)
 {
     SwapDir *sd = INDEXSD(i);
     assert(sd);
@@ -209,7 +209,7 @@ Store::Disks::create()
     }
 
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        if (dir(i).active())
+        if (Dir(i).active())
             store(i)->create();
     }
 }
@@ -283,7 +283,7 @@ Store::Disks::init()
         *         above
         * Step 3: have the hash index walk the searches itself.
          */
-        if (dir(i).active())
+        if (Dir(i).active())
             store(i)->init();
     }
 
@@ -302,7 +302,7 @@ Store::Disks::maxSize() const
     uint64_t result = 0;
 
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        if (dir(i).doReportStat())
+        if (Dir(i).doReportStat())
             result += store(i)->maxSize();
     }
 
@@ -315,7 +315,7 @@ Store::Disks::minSize() const
     uint64_t result = 0;
 
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        if (dir(i).doReportStat())
+        if (Dir(i).doReportStat())
             result += store(i)->minSize();
     }
 
@@ -328,7 +328,7 @@ Store::Disks::currentSize() const
     uint64_t result = 0;
 
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        if (dir(i).doReportStat())
+        if (Dir(i).doReportStat())
             result += store(i)->currentSize();
     }
 
@@ -341,7 +341,7 @@ Store::Disks::currentCount() const
     uint64_t result = 0;
 
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        if (dir(i).doReportStat())
+        if (Dir(i).doReportStat())
             result += store(i)->currentCount();
     }
 
@@ -362,7 +362,7 @@ Store::Disks::updateLimits()
     secondLargestMaximumObjectSize = -1;
 
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        const auto &disk = dir(i);
+        const auto &disk = Dir(i);
         if (!disk.active())
             continue;
 
@@ -506,8 +506,8 @@ void
 Store::Disks::evictIfFound(const cache_key *key)
 {
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        if (dir(i).active())
-            dir(i).evictIfFound(key);
+        if (Dir(i).active())
+            Dir(i).evictIfFound(key);
     }
 }
 
@@ -521,7 +521,7 @@ Store::Disks::anchorToCache(StoreEntry &entry, bool &inSync)
         static int idx = 0;
         for (int n = 0; n < cacheDirs; ++n) {
             idx = (idx + 1) % cacheDirs;
-            SwapDir &sd = dir(idx);
+            SwapDir &sd = Dir(idx);
             if (!sd.active())
                 continue;
 
@@ -541,17 +541,17 @@ bool
 Store::Disks::updateAnchored(StoreEntry &entry)
 {
     return entry.hasDisk() &&
-           dir(entry.swap_dirn).updateAnchored(entry);
+           Dir(entry.swap_dirn).updateAnchored(entry);
 }
 
 bool
-Store::Disks::smpAware() const
+Store::Disks::SmpAware()
 {
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
         // A mix is not supported, but we conservatively check every
         // dir because features like collapsed revalidation should
         // currently be disabled if any dir is SMP-aware
-        if (dir(i).smpAware())
+        if (Dir(i).smpAware())
             return true;
     }
     return false;
@@ -561,7 +561,7 @@ bool
 Store::Disks::hasReadableEntry(const StoreEntry &e) const
 {
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i)
-        if (dir(i).active() && dir(i).hasReadableEntry(e))
+        if (Dir(i).active() && Dir(i).hasReadableEntry(e))
             return true;
     return false;
 }

diff --git a/src/store/Disks.h b/src/store/Disks.h
index 7a6a12ca88c52b5bfa4872a2fe5107ce4303c846..d1b83eac53775b0f109e1ca77738d61a600bd629 100644 (file)
--- a/src/store/Disks.h
+++ b/src/store/Disks.h
@@ -48,14 +48,15 @@ public:
     /// Additional unknown-size entry bytes required by disks in order to
     /// reduce the risk of selecting the wrong disk cache for the growing entry.
     int64_t accumulateMore(const StoreEntry&) const;
-    virtual bool smpAware() const override;
+    /// whether any disk cache is SMP-aware
+    static bool SmpAware();
     /// whether any of disk caches has entry with e.key
     bool hasReadableEntry(const StoreEntry &) const;
 
 private:
     /* migration logic */
     SwapDir *store(int const x) const;
-    SwapDir &dir(int const idx) const;
+    static SwapDir &Dir(int const idx);
 
     int64_t largestMinimumObjectSize; ///< maximum of all Disk::minObjectSize()s
     int64_t largestMaximumObjectSize; ///< maximum of all Disk::maxObjectSize()s

diff --git a/src/store/Storage.h b/src/store/Storage.h
index 5dd9410649ec365b4e20f9e8e1776732ee2d2640..1a16964770dc34643d11cfa96314e195f0f6b32b 100644 (file)
--- a/src/store/Storage.h
+++ b/src/store/Storage.h
@@ -76,11 +76,6 @@ public:
 
     /// prepare for shutdown
     virtual void sync() {}
-
-    /// whether this storage is capable of serving multiple workers;
-    /// a true result does not imply [lack of] non-SMP support because
-    /// [only] some SMP-aware storages also support non-SMP configss
-    virtual bool smpAware() const = 0;
 };
 
 } // namespace Store

diff --git a/src/tests/TestSwapDir.h b/src/tests/TestSwapDir.h
index a1b0869563c2340d28008d55976d981a426330c2..9531eabe2bc3e1acceb2ae351a9f5b9dece795eb 100644 (file)
--- a/src/tests/TestSwapDir.h
+++ b/src/tests/TestSwapDir.h
@@ -36,6 +36,7 @@ public:
     virtual void evictCached(StoreEntry &) override {}
     virtual void evictIfFound(const cache_key *) override {}
     virtual bool hasReadableEntry(const StoreEntry &) const override { return false; }
+    virtual bool smpAware() const override { return false; }
 };
 
 typedef RefCount<TestSwapDir> TestSwapDirPointer;

diff --git a/src/tests/stub_MemStore.cc b/src/tests/stub_MemStore.cc
index 10d9de7078a70d76e55c6e8c132ae1a7b61fed51..eacc0349bd883230739646e0cd3a596d1144e161 100644 (file)
--- a/src/tests/stub_MemStore.cc
+++ b/src/tests/stub_MemStore.cc
@@ -38,4 +38,5 @@ void MemStore::evictCached(StoreEntry&) STUB
 void MemStore::evictIfFound(const cache_key *) STUB
 bool MemStore::anchorToCache(StoreEntry&, bool&) STUB_RETVAL(false)
 bool MemStore::updateAnchored(StoreEntry&) STUB_RETVAL(false)
+int64_t MemStore::EntryLimit() STUB_RETVAL(0)
 

diff --git a/src/tests/stub_store.cc b/src/tests/stub_store.cc
index 0fc07d761f19e98bd2b9c61067a4fa540d1fdf15..22281872850170b0a050514567d7331285a3b6f8 100644 (file)
--- a/src/tests/stub_store.cc
+++ b/src/tests/stub_store.cc
@@ -113,6 +113,7 @@ StoreSearch *Store::Controller::search() STUB_RETVAL(NULL)
 void Store::Controller::maintain() STUB
 bool Store::Controller::markedForDeletion(const cache_key *) const STUB_RETVAL(false)
 void Store::Controller::freeMemorySpace(const int) STUB
+bool Store::Controller::SmpAware() STUB_RETVAL(false)
 
 std::ostream &operator <<(std::ostream &os, const StoreEntry &)
 {