Hello,

    The attached patch allows a ufs cache_dir entry to coexist with a
shared memory cache entry instead of being released when it becomes idle.

The original boolean version of the StoreController::dereference() code
(r11730) was written to make sure that idle unlocked local store_table
entries are released if nobody needs them (to avoid creating
inconsistencies with shared caches that could be modified in a different
process).

Then, in r11786, we realized that the original code was destroying
non-shared memory cache entries if there were no cache_dirs to vote for
keeping them in store_table. I fixed that by changing the
StoreController::dereference() logic from "remove if nobody needs it" to
"remove if somebody objects to keeping it". That solved the problem at
hand, but prohibited an entry to exist in a non-shared cache_dir and in
a shared memory cache at the same time.

We now go back to the original "remove if nobody needs it" design but
also give non-shared memory cache a vote so that it can protect idle but
suitable for memory cache entries from being released if there are no
cache_dirs to vote for them.


This is a second revision of the fix. The first one (trunk r12231) was
reverted because it did not pass tests/testRock unit tests on some
platforms. The unit tests assume that the entry slot is not locked after
the entry is stored, but the first revision of the fix allowed idle
entries to remain in store_table and, hence, their slots were locked and
could not be replaced, causing testRock assertions.  This revision
allows the idle entry to be destroyed (and its slot unlocked) if
[non-shared] memory caching is disabled.

It is not clear to me why only some of the platforms were affected by
this. Should not memory caching be disabled everywhere during testRock
(because testRock does not set memory cache capacity and memory cache
entry size limits)?

The changes passed build farm tests (except a couple of nodes with
Jenkins-related build problems unrelated to the code).


Thank you,

Alex.
Allow a ufs cache_dir entry to coexist with a shared memory cache entry
instead of being released when it becomes idle.

The original boolean version of the StoreController::dereference() code
(r11730) was written to make sure that idle unlocked local store_table entries
are released if nobody needs them (to avoid creating inconsistencies with
shared caches that could be modified in a different process).

Then, in r11786, we realized that the original code was destroying non-shared
memory cache entries if there were no cache_dirs to vote for keeping them in
store_table. I fixed that by changing the StoreController::dereference() logic
from "remove if nobody needs it" to "remove if somebody objects to keeping
it". That solved the problem at hand, but prohibited an entry to exist in
a non-shared cache_dir and in a shared memory cache at the same time.

We now go back to the original "remove if nobody needs it" design but also
give non-shared memory cache a vote so that it can protect idle but suitable
for memory cache entries from being released if there are no cache_dirs to
vote for them.


This is a second revision of the fix. The first one (r12231) was reverted
because it did not pass tests/testRock unit tests on some platforms. The unit
tests assume that the entry slot is not locked after the entry is stored, but
the first revision of the fix allowed idle entries to remain in store_table
and, hence, their slots were locked and could not be replaced, causing
assertions.  This revision allows the idle entry to be destroyed (and its slot
unlocked) if [non-shared] memory caching is disabled.

It is not clear why only some of the platforms were affected by this. Should
not memory caching be disabled everywhere during testRock (because testRock
does not set memory cache capacity and memory cache entry size limits)?

=== modified file 'src/MemStore.cc'
--- src/MemStore.cc	2012-09-04 09:10:20 +0000
+++ src/MemStore.cc	2012-10-10 19:04:30 +0000
@@ -113,41 +113,41 @@
 }
 
 uint64_t
 MemStore::currentCount() const
 {
     return map ? map->entryCount() : 0;
 }
 
 int64_t
 MemStore::maxObjectSize() const
 {
     return Ipc::Mem::PageSize();
 }
 
 void
 MemStore::reference(StoreEntry &)
 {
 }
 
 bool
-MemStore::dereference(StoreEntry &)
+MemStore::dereference(StoreEntry &, bool)
 {
     // no need to keep e in the global store_table for us; we have our own map
     return false;
 }
 
 int
 MemStore::callback()
 {
     return 0;
 }
 
 StoreSearch *
 MemStore::search(String const, HttpRequest *)
 {
     fatal("not implemented");
     return NULL;
 }
 
 StoreEntry *
 MemStore::get(const cache_key *key)

=== modified file 'src/MemStore.h'
--- src/MemStore.h	2012-10-04 11:10:17 +0000
+++ src/MemStore.h	2012-10-10 19:04:22 +0000
@@ -23,41 +23,41 @@
     /// cache the entry or forget about it until the next considerKeeping call
     void considerKeeping(StoreEntry &e);
 
     /// whether e should be kept in local RAM for possible future caching
     bool keepInLocalMemory(const StoreEntry &e) const;
 
     /* Store API */
     virtual int callback();
     virtual StoreEntry * get(const cache_key *);
     virtual void get(String const key , STOREGETCLIENT callback, void *cbdata);
     virtual void init();
     virtual uint64_t maxSize() const;
     virtual uint64_t minSize() const;
     virtual uint64_t currentSize() const;
     virtual uint64_t currentCount() const;
     virtual int64_t maxObjectSize() const;
     virtual void getStats(StoreInfoStats &stats) const;
     virtual void stat(StoreEntry &) const;
     virtual StoreSearch *search(String const url, HttpRequest *);
     virtual void reference(StoreEntry &);
-    virtual bool dereference(StoreEntry &);
+    virtual bool dereference(StoreEntry &, bool);
     virtual void maintain();
 
     static int64_t EntryLimit();
 
 protected:
     bool willFit(int64_t needed) const;
     void keep(StoreEntry &e);
 
     bool copyToShm(StoreEntry &e, MemStoreMap::Extras &extras);
     bool copyFromShm(StoreEntry &e, const MemStoreMap::Extras &extras);
 
     // Ipc::StoreMapCleaner API
     virtual void cleanReadable(const sfileno fileno);
 
 private:
     MemStoreMap *map; ///< index of mem-cached entries
     uint64_t theCurrentSize; ///< currently used space in the storage area
 };
 
 // Why use Store as a base? MemStore and SwapDir are both "caches".

=== modified file 'src/Store.h'
--- src/Store.h	2012-09-22 20:07:31 +0000
+++ src/Store.h	2012-10-10 18:35:48 +0000
@@ -324,41 +324,41 @@
 
     /**
      * Output stats to the provided store entry.
      \todo make these calls asynchronous
      */
     virtual void stat(StoreEntry &) const = 0;
 
     /** Sync the store prior to shutdown */
     virtual void sync();
 
     /** remove a Store entry from the store */
     virtual void unlink (StoreEntry &);
 
     /* search in the store */
     virtual StoreSearch *search(String const url, HttpRequest *) = 0;
 
     /* pulled up from SwapDir for migration.... probably do not belong here */
     virtual void reference(StoreEntry &) = 0;	/* Reference this object */
 
     /// Undo reference(), returning false iff idle e should be destroyed
-    virtual bool dereference(StoreEntry &e) = 0;
+    virtual bool dereference(StoreEntry &e, bool wantsLocalMemory) = 0;
 
     virtual void maintain() = 0; /* perform regular maintenance should be private and self registered ... */
 
     // XXX: This method belongs to Store::Root/StoreController, but it is here
     // because test cases use non-StoreController derivatives as Root
     /// called when the entry is no longer needed by any transaction
     virtual void handleIdleEntry(StoreEntry &e) {}
 
     // XXX: This method belongs to Store::Root/StoreController, but it is here
     // because test cases use non-StoreController derivatives as Root
     /// called to get rid of no longer needed entry data in RAM, if any
     virtual void maybeTrimMemory(StoreEntry &e, const bool preserveSwappable) {}
 
 private:
     static RefCount<Store> CurrentRoot;
 };
 
 /// \ingroup StoreAPI
 typedef RefCount<Store> StorePointer;
 

=== modified file 'src/StoreHashIndex.h'
--- src/StoreHashIndex.h	2012-09-01 14:38:36 +0000
+++ src/StoreHashIndex.h	2012-10-10 18:35:48 +0000
@@ -59,41 +59,41 @@
 
     virtual void init();
 
     virtual void sync();
 
     virtual uint64_t maxSize() const;
 
     virtual uint64_t minSize() const;
 
     virtual uint64_t currentSize() const;
 
     virtual uint64_t currentCount() const;
 
     virtual int64_t maxObjectSize() const;
 
     virtual void getStats(StoreInfoStats &stats) const;
     virtual void stat(StoreEntry&) const;
 
     virtual void reference(StoreEntry&);
 
-    virtual bool dereference(StoreEntry&);
+    virtual bool dereference(StoreEntry&, bool);
 
     virtual void maintain();
 
     virtual StoreSearch *search(String const url, HttpRequest *);
 
 private:
     /* migration logic */
     StorePointer store(int const x) const;
     SwapDir &dir(int const idx) const;
 };
 
 class StoreHashIndexEntry : public StoreEntry
     {};
 
 class StoreSearchHashIndex : public StoreSearch
 {
 
 public:
     StoreSearchHashIndex(RefCount<StoreHashIndex> sd);
     StoreSearchHashIndex(StoreSearchHashIndex const &);

=== modified file 'src/SwapDir.cc'
--- src/SwapDir.cc	2012-09-04 09:10:20 +0000
+++ src/SwapDir.cc	2012-10-10 18:36:29 +0000
@@ -101,41 +101,41 @@
             repl->Stats(repl, &output);
     }
 }
 
 void
 SwapDir::statfs(StoreEntry &)const {}
 
 void
 SwapDir::maintain() {}
 
 uint64_t
 SwapDir::minSize() const
 {
     return ((maxSize() * Config.Swap.lowWaterMark) / 100);
 }
 
 void
 SwapDir::reference(StoreEntry &) {}
 
 bool
-SwapDir::dereference(StoreEntry &)
+SwapDir::dereference(StoreEntry &, bool)
 {
     return true; // keep in global store_table
 }
 
 int
 SwapDir::callback()
 {
     return 0;
 }
 
 bool
 SwapDir::canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const
 {
     debugs(47,8, HERE << "cache_dir[" << index << "]: needs " <<
            diskSpaceNeeded << " <? " << max_objsize);
 
     if (EBIT_TEST(e.flags, ENTRY_SPECIAL))
         return false; // we do not store Squid-generated entries
 
     if (!objectSizeIsAcceptable(diskSpaceNeeded))

=== modified file 'src/SwapDir.h'
--- src/SwapDir.h	2012-09-21 14:57:30 +0000
+++ src/SwapDir.h	2012-10-10 18:35:48 +0000
@@ -67,41 +67,41 @@
 
     virtual uint64_t maxSize() const;
 
     virtual uint64_t minSize() const;
 
     virtual uint64_t currentSize() const;
 
     virtual uint64_t currentCount() const;
 
     virtual int64_t maxObjectSize() const;
 
     virtual void getStats(StoreInfoStats &stats) const;
     virtual void stat(StoreEntry &) const;
 
     virtual void sync();	/* Sync the store prior to shutdown */
 
     virtual StoreSearch *search(String const url, HttpRequest *);
 
     virtual void reference(StoreEntry &);	/* Reference this object */
 
-    virtual bool dereference(StoreEntry &);	/* Unreference this object */
+    virtual bool dereference(StoreEntry &, bool);	/* Unreference this object */
 
     /* the number of store dirs being rebuilt. */
     static int store_dirs_rebuilding;
 
 private:
     void createOneStore(Store &aStore);
     bool keepForLocalMemoryCache(const StoreEntry &e) const;
 
     StorePointer swapDir; ///< summary view of all disk caches
     MemStore *memStore; ///< memory cache
 };
 
 /* migrating from the Config based list of swapdirs */
 void allocate_new_swapdir(SquidConfig::_cacheSwap *);
 void free_cachedir(SquidConfig::_cacheSwap * swap);
 extern OBJH storeDirStats;
 char *storeDirSwapLogFile(int, const char *);
 char *storeSwapFullPath(int, char *);
 char *storeSwapSubSubDir(int, char *);
 const char *storeSwapPath(int);
@@ -189,41 +189,41 @@
     int64_t max_objsize;
     RemovalPolicy *repl;
     int removals;
     int scanned;
 
     struct Flags {
         Flags() : selected(0), read_only(0) {}
         unsigned int selected:1;
         unsigned int read_only:1;
     } flags;
     virtual void init() = 0;	/* Initialise the fs */
     virtual void create();	/* Create a new fs */
     virtual void dump(StoreEntry &)const;	/* Dump fs config snippet */
     virtual bool doubleCheck(StoreEntry &);	/* Double check the obj integrity */
     virtual void statfs(StoreEntry &) const;	/* Dump fs statistics */
     virtual void maintain();	/* Replacement maintainence */
     /// check whether we can store the entry; if we can, report current load
     virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const = 0;
     /* These two are notifications */
     virtual void reference(StoreEntry &);	/* Reference this object */
-    virtual bool dereference(StoreEntry &);	/* Unreference this object */
+    virtual bool dereference(StoreEntry &, bool);	/* Unreference this object */
     virtual int callback();	/* Handle pending callbacks */
     virtual void sync();	/* Sync the store prior to shutdown */
     virtual StoreIOState::Pointer createStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *) = 0;
     virtual StoreIOState::Pointer openStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *) = 0;
     virtual void unlink (StoreEntry &);
     bool canLog(StoreEntry const &e)const;
     virtual void openLog();
     virtual void closeLog();
     virtual void logEntry(const StoreEntry & e, int op) const;
 
     class CleanLog
     {
 
     public:
         virtual ~CleanLog() {}
 
         virtual const StoreEntry *nextEntry() = 0;
         virtual void write(StoreEntry const &) = 0;
     };
 

=== modified file 'src/fs/rock/RockSwapDir.cc'
--- src/fs/rock/RockSwapDir.cc	2012-09-04 09:10:20 +0000
+++ src/fs/rock/RockSwapDir.cc	2012-10-10 18:37:14 +0000
@@ -729,41 +729,41 @@
     debugs(47,2, HERE << "Rock cache_dir[" << index << "] freed " << freed <<
            " scanned " << walker->scanned << '/' << walker->locked);
 
     walker->Done(walker);
 
     if (full()) {
         debugs(47, DBG_CRITICAL, "ERROR: Rock cache_dir[" << index << "] " <<
                "is still full after freeing " << freed << " entries. A bug?");
     }
 }
 
 void
 Rock::SwapDir::reference(StoreEntry &e)
 {
     debugs(47, 5, HERE << &e << ' ' << e.swap_dirn << ' ' << e.swap_filen);
     if (repl && repl->Referenced)
         repl->Referenced(repl, &e, &e.repl);
 }
 
 bool
-Rock::SwapDir::dereference(StoreEntry &e)
+Rock::SwapDir::dereference(StoreEntry &e, bool)
 {
     debugs(47, 5, HERE << &e << ' ' << e.swap_dirn << ' ' << e.swap_filen);
     if (repl && repl->Dereferenced)
         repl->Dereferenced(repl, &e, &e.repl);
 
     // no need to keep e in the global store_table for us; we have our own map
     return false;
 }
 
 bool
 Rock::SwapDir::unlinkdUseful() const
 {
     // no entry-specific files to unlink
     return false;
 }
 
 void
 Rock::SwapDir::unlink(StoreEntry &e)
 {
     debugs(47, 5, HERE << e);

=== modified file 'src/fs/rock/RockSwapDir.h'
--- src/fs/rock/RockSwapDir.h	2012-08-28 13:00:30 +0000
+++ src/fs/rock/RockSwapDir.h	2012-10-10 18:38:25 +0000
@@ -36,41 +36,41 @@
     virtual void create();
     virtual void parse(int index, char *path);
 
     int64_t entryLimitHigh() const { return SwapFilenMax; } ///< Core limit
     int64_t entryLimitAllowed() const;
 
     typedef Ipc::StoreMapWithExtras<DbCellHeader> DirMap;
 
 protected:
     /* protected ::SwapDir API */
     virtual bool needsDiskStrand() const;
     virtual void init();
     virtual ConfigOption *getOptionTree() const;
     virtual bool allowOptionReconfigure(const char *const option) const;
     virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const;
     virtual StoreIOState::Pointer createStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual StoreIOState::Pointer openStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual void maintain();
     virtual void diskFull();
     virtual void reference(StoreEntry &e);
-    virtual bool dereference(StoreEntry &e);
+    virtual bool dereference(StoreEntry &e, bool);
     virtual bool unlinkdUseful() const;
     virtual void unlink(StoreEntry &e);
     virtual void statfs(StoreEntry &e) const;
 
     /* IORequestor API */
     virtual void ioCompletedNotification();
     virtual void closeCompleted();
     virtual void readCompleted(const char *buf, int len, int errflag, RefCount< ::ReadRequest>);
     virtual void writeCompleted(int errflag, size_t len, RefCount< ::WriteRequest>);
 
     void parseSize(const bool reconfiguring); ///< parses anonymous cache_dir size option
     void validateOptions(); ///< warns of configuration problems; may quit
     bool parseTimeOption(char const *option, const char *value, int reconfiguring);
     void dumpTimeOption(StoreEntry * e) const;
     bool parseRateOption(char const *option, const char *value, int reconfiguring);
     void dumpRateOption(StoreEntry * e) const;
 
     void rebuild(); ///< starts loading and validating stored entry metadata
     ///< used to add entries successfully loaded during rebuild
     bool addEntry(const int fileno, const DbCellHeader &header, const StoreEntry &from);

=== modified file 'src/fs/ufs/RebuildState.cc'
--- src/fs/ufs/RebuildState.cc	2012-09-04 09:10:20 +0000
+++ src/fs/ufs/RebuildState.cc	2012-10-10 18:38:16 +0000
@@ -329,41 +329,41 @@
     /* If this URL already exists in the cache, does the swap log
      * appear to have a newer entry?  Compare 'lastref' from the
      * swap log to e->lastref. */
     /* is the log entry newer than current entry? */
     int disk_entry_newer = currentEntry() ? (swapData.lastref > currentEntry()->lastref ? 1 : 0) : 0;
 
     if (used && !disk_entry_newer) {
         /* log entry is old, ignore it */
         ++counts.clashcount;
         return;
     } else if (used && currentEntry() && currentEntry()->swap_filen == swapData.swap_filen && currentEntry()->swap_dirn == sd->index) {
         /* swapfile taken, same URL, newer, update meta */
 
         if (currentEntry()->store_status == STORE_OK) {
             currentEntry()->lastref = swapData.timestamp;
             currentEntry()->timestamp = swapData.timestamp;
             currentEntry()->expires = swapData.expires;
             currentEntry()->lastmod = swapData.lastmod;
             currentEntry()->flags = swapData.flags;
             currentEntry()->refcount += swapData.refcount;
-            sd->dereference(*currentEntry());
+            sd->dereference(*currentEntry(), false);
         } else {
             debug_trap("commonUfsDirRebuildFromSwapLog: bad condition");
             debugs(47, DBG_IMPORTANT, HERE << "bad condition");
         }
         return;
     } else if (used) {
         /* swapfile in use, not by this URL, log entry is newer */
         /* This is sorta bad: the log entry should NOT be newer at this
          * point.  If the log is dirty, the filesize check should have
          * caught this.  If the log is clean, there should never be a
          * newer entry. */
         debugs(47, DBG_IMPORTANT, "WARNING: newer swaplog entry for dirno " <<
                sd->index  << ", fileno "<< std::setfill('0') << std::hex <<
                std::uppercase << std::setw(8) << swapData.swap_filen);
 
         /* I'm tempted to remove the swapfile here just to be safe,
          * but there is a bad race condition in the NOVM version if
          * the swapfile has recently been opened for writing, but
          * not yet opened for reading.  Because we can't map
          * swapfiles back to StoreEntrys, we don't know the state

=== modified file 'src/fs/ufs/UFSSwapDir.cc'
--- src/fs/ufs/UFSSwapDir.cc	2012-10-08 05:35:27 +0000
+++ src/fs/ufs/UFSSwapDir.cc	2012-10-10 18:37:14 +0000
@@ -474,41 +474,41 @@
         e->release();
     }
 
     walker->Done(walker);
     debugs(47, (removed ? 2 : 3), HERE << path <<
            " removed " << removed << "/" << max_remove << " f=" <<
            std::setprecision(4) << f << " max_scan=" << max_scan);
 }
 
 void
 Fs::Ufs::UFSSwapDir::reference(StoreEntry &e)
 {
     debugs(47, 3, HERE << "referencing " << &e << " " <<
            e.swap_dirn << "/" << e.swap_filen);
 
     if (repl->Referenced)
         repl->Referenced(repl, &e, &e.repl);
 }
 
 bool
-Fs::Ufs::UFSSwapDir::dereference(StoreEntry & e)
+Fs::Ufs::UFSSwapDir::dereference(StoreEntry & e, bool)
 {
     debugs(47, 3, HERE << "dereferencing " << &e << " " <<
            e.swap_dirn << "/" << e.swap_filen);
 
     if (repl->Dereferenced)
         repl->Dereferenced(repl, &e, &e.repl);
 
     return true; // keep e in the global store_table
 }
 
 StoreIOState::Pointer
 Fs::Ufs::UFSSwapDir::createStoreIO(StoreEntry &e, StoreIOState::STFNCB * file_callback, StoreIOState::STIOCB * aCallback, void *callback_data)
 {
     return IO->create (this, &e, file_callback, aCallback, callback_data);
 }
 
 StoreIOState::Pointer
 Fs::Ufs::UFSSwapDir::openStoreIO(StoreEntry &e, StoreIOState::STFNCB * file_callback, StoreIOState::STIOCB * aCallback, void *callback_data)
 {
     return IO->open (this, &e, file_callback, aCallback, callback_data);

=== modified file 'src/fs/ufs/UFSSwapDir.h'
--- src/fs/ufs/UFSSwapDir.h	2012-08-10 06:56:49 +0000
+++ src/fs/ufs/UFSSwapDir.h	2012-10-10 18:38:16 +0000
@@ -78,41 +78,41 @@
     virtual void unlink(StoreEntry &);
     virtual void statfs(StoreEntry &)const;
     virtual void maintain();
     /** check whether this filesystem can store the given object
      *
      * UFS filesystems will happily store anything as long as
      * the LRU time isn't too small
      */
     virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const;
     /** reference an object
      *
      * This routine is called whenever an object is referenced, so we can
      * maintain replacement information within the storage fs.
      */
     virtual void reference(StoreEntry &);
     /** de-reference an object
      *
      * This routine is called whenever the last reference to an object is
      * removed, to maintain replacement information within the storage fs.
      */
-    virtual bool dereference(StoreEntry &);
+    virtual bool dereference(StoreEntry &, bool);
     virtual StoreIOState::Pointer createStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual StoreIOState::Pointer openStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual void openLog();
     virtual void closeLog();
     virtual int writeCleanStart();
     virtual void writeCleanDone();
     virtual void logEntry(const StoreEntry & e, int op) const;
     virtual void parse(int index, char *path); ///parse configuration and setup new SwapDir
     virtual void reconfigure(); ///reconfigure the SwapDir
     virtual int callback();
     virtual void sync();
     virtual void swappedOut(const StoreEntry &e);
     virtual uint64_t currentSize() const { return cur_size; }
     virtual uint64_t currentCount() const { return n_disk_objects; }
 
     void unlinkFile(sfileno f);
     // move down when unlink is a virtual method
     //protected:
     Fs::Ufs::UFSStrategy *IO;
     char *fullPath(sfileno, char *) const;

=== modified file 'src/store_dir.cc'
--- src/store_dir.cc	2012-10-08 05:21:11 +0000
+++ src/store_dir.cc	2012-10-10 19:05:23 +0000
@@ -695,61 +695,64 @@
     if (EBIT_TEST(e.flags, ENTRY_SPECIAL))
         return;
 
     /* Notify the fs that we're referencing this object again */
 
     if (e.swap_dirn > -1)
         swapDir->reference(e);
 
     // Notify the memory cache that we're referencing this object again
     if (memStore && e.mem_status == IN_MEMORY)
         memStore->reference(e);
 
     // TODO: move this code to a non-shared memory cache class when we have it
     if (e.mem_obj) {
         if (mem_policy->Referenced)
             mem_policy->Referenced(mem_policy, &e, &e.mem_obj->repl);
     }
 }
 
 bool
-StoreController::dereference(StoreEntry & e)
+StoreController::dereference(StoreEntry &e, bool wantsLocalMemory)
 {
-    bool keepInStoreTable = true; // keep if there are no objections
-
     // special entries do not belong to any specific Store, but are IN_MEMORY
     if (EBIT_TEST(e.flags, ENTRY_SPECIAL))
-        return keepInStoreTable;
+        return true;
+
+    bool keepInStoreTable = false; // keep only if somebody needs it there
 
     /* Notify the fs that we're not referencing this object any more */
 
     if (e.swap_filen > -1)
-        keepInStoreTable = swapDir->dereference(e) && keepInStoreTable;
+        keepInStoreTable = swapDir->dereference(e, wantsLocalMemory) || 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;
+        keepInStoreTable = memStore->dereference(e, wantsLocalMemory) || 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)
+            keepInStoreTable = wantsLocalMemory || keepInStoreTable;
     }
 
     return keepInStoreTable;
 }
 
 StoreEntry *
 StoreController::get(const cache_key *key)
 {
     if (StoreEntry *e = swapDir->get(key)) {
         // TODO: ignore and maybe handleIdleEntry() unlocked intransit entries
         // because their backing store slot may be gone already.
         debugs(20, 3, HERE << "got in-transit entry: " << *e);
         return e;
     }
 
     if (memStore) {
         if (StoreEntry *e = memStore->get(key)) {
             debugs(20, 3, HERE << "got mem-cached entry: " << *e);
             return e;
         }
@@ -823,43 +826,43 @@
 
 void
 StoreController::handleIdleEntry(StoreEntry &e)
 {
     bool keepInLocalMemory = false;
 
     if (EBIT_TEST(e.flags, ENTRY_SPECIAL)) {
         // Icons (and cache digests?) should stay in store_table until we
         // 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) {
         memStore->considerKeeping(e);
         // leave keepInLocalMemory false; memStore maintains its own cache
     } else {
         keepInLocalMemory = keepForLocalMemoryCache(e) && // in good shape and
                             // the local memory cache is not overflowing
                             (mem_node::InUseCount() <= store_pages_max);
     }
 
-    // An idle, unlocked entry that belongs to a SwapDir which controls
+    // An idle, unlocked entry that only belongs to a SwapDir which controls
     // its own index, should not stay in the global store_table.
-    if (!dereference(e)) {
+    if (!dereference(e, keepInLocalMemory)) {
         debugs(20, 5, HERE << "destroying unlocked entry: " << &e << ' ' << e);
         destroyStoreEntry(static_cast<hash_link*>(&e));
         return;
     }
 
     debugs(20, 5, HERE << "keepInLocalMemory: " << keepInLocalMemory);
 
     // TODO: move this into [non-shared] memory cache class when we have one
     if (keepInLocalMemory) {
         e.setMemStatus(IN_MEMORY);
         e.mem_obj->unlinkRequest();
     } else {
         e.purgeMem(); // may free e
     }
 }
 
 StoreHashIndex::StoreHashIndex()
 {
     if (store_table)
         abort();
@@ -1060,43 +1063,43 @@
 void
 StoreHashIndex::stat(StoreEntry & output) const
 {
     int i;
 
     /* Now go through each store, calling its stat routine */
 
     for (i = 0; i < Config.cacheSwap.n_configured; ++i) {
         storeAppendPrintf(&output, "\n");
         store(i)->stat(output);
     }
 }
 
 void
 StoreHashIndex::reference(StoreEntry &e)
 {
     e.store()->reference(e);
 }
 
 bool
-StoreHashIndex::dereference(StoreEntry &e)
+StoreHashIndex::dereference(StoreEntry &e, bool wantsLocalMemory)
 {
-    return e.store()->dereference(e);
+    return e.store()->dereference(e, wantsLocalMemory);
 }
 
 void
 StoreHashIndex::maintain()
 {
     int i;
     /* walk each fs */
 
     for (i = 0; i < Config.cacheSwap.n_configured; ++i) {
         /* XXX FixMe: This should be done "in parallell" on the different
          * cache_dirs, not one at a time.
          */
         /* call the maintain function .. */
         store(i)->maintain();
     }
 }
 
 void
 StoreHashIndex::sync()
 {

=== modified file 'src/tests/stub_MemStore.cc'
--- src/tests/stub_MemStore.cc	2012-09-01 14:38:36 +0000
+++ src/tests/stub_MemStore.cc	2012-10-10 18:37:14 +0000
@@ -11,21 +11,21 @@
 
 MemStore::MemStore() STUB
 MemStore::~MemStore() STUB
 bool MemStore::keepInLocalMemory(const StoreEntry &) const STUB_RETVAL(false)
 void MemStore::considerKeeping(StoreEntry &) STUB
 void MemStore::reference(StoreEntry &) STUB
 void MemStore::maintain() STUB
 void MemStore::cleanReadable(const sfileno) STUB
 void MemStore::get(String const, STOREGETCLIENT, void *) STUB
 void MemStore::init() STUB
 void MemStore::getStats(StoreInfoStats&) const STUB
 void MemStore::stat(StoreEntry &) const STUB
 int MemStore::callback() STUB_RETVAL(0)
 StoreEntry *MemStore::get(const cache_key *) STUB_RETVAL(NULL)
 uint64_t MemStore::maxSize() const STUB_RETVAL(0)
 uint64_t MemStore::minSize() const STUB_RETVAL(0)
 uint64_t MemStore::currentSize() const STUB_RETVAL(0)
 uint64_t MemStore::currentCount() const STUB_RETVAL(0)
 int64_t MemStore::maxObjectSize() const STUB_RETVAL(0)
 StoreSearch *MemStore::search(String const, HttpRequest *) STUB_RETVAL(NULL)
-bool MemStore::dereference(StoreEntry &) STUB_RETVAL(false)
+bool MemStore::dereference(StoreEntry &, bool) STUB_RETVAL(false)

=== modified file 'src/tests/testStore.h'
--- src/tests/testStore.h	2012-08-28 13:00:30 +0000
+++ src/tests/testStore.h	2012-10-10 19:08:37 +0000
@@ -49,29 +49,29 @@
     virtual void init();
 
     virtual void maintain() {};
 
     virtual uint64_t maxSize() const;
 
     virtual uint64_t minSize() const;
 
     virtual uint64_t currentSize() const;
 
     virtual uint64_t currentCount() const;
 
     virtual int64_t maxObjectSize() const;
 
     virtual void getStats(StoreInfoStats &) const;
 
     virtual void stat(StoreEntry &) const; /* output stats to the provided store entry */
 
     virtual void reference(StoreEntry &) {}	/* Reference this object */
 
-    virtual bool dereference(StoreEntry &) { return true; }
+    virtual bool dereference(StoreEntry &, bool) { return true; }
 
     virtual StoreSearch *search(String const url, HttpRequest *);
 };
 
 typedef RefCount<TestStore> TestStorePointer;
 
 #endif
 

Reply via email to