Since r11969, Squid calls trimMemory() for all entries, including
non-swappable, to release unused MemObjects memory. But it should not
release objects that are or should be stored in local memory cache.
StoreEntry::trimMemory() has a check for IN_MEMORY status for that. But
IN_MEMORY status is set too late in StoreController::handleIdleEntry(),
after trimMemory() marks entry for release:
clientReplyContext::removeStoreReference()
storeUnregister()
StoreEntry::swapOut()
StoreEntry::trimMemory()
StoreEntry::releaseRequest()
StoreEntry::unlock()
StoreController::handleIdleEntry() // never get here because entry is
set IN_MEMORY status // already marked for release
The patch adds StoreController::keepInLocalMemory() method to determine
if an entry should be kept in memory for later use. It uses different
checks depending on the configuration: shared memory cache checks are
implemented in MemStore::keepInLocalMemory() and local ones are in
StoreController::keepInLocalMemoryCache(). Both methods use
StoreEntry::memoryCachable() for general checks. Shared memory
cache-specific checks are moved from StoreEntry::memoryCachable() to
MemStore::keepInLocalMemory().
Regards,
Dmitry
Do not release entries that may be stored in memory cache later.
Since r11969, Squid calls trimMemory() for all entries, including
non-swappable, to release unused MemObjects memory. But it should not
release objects that are or should be stored in local memory cache.
StoreEntry::trimMemory() has a check for IN_MEMORY status for that. But
IN_MEMORY status is set too late in StoreController::handleIdleEntry(),
after trimMemory() marks entry for release:
clientReplyContext::removeStoreReference()
storeUnregister()
StoreEntry::swapOut()
StoreEntry::trimMemory()
StoreEntry::releaseRequest()
StoreEntry::unlock()
StoreController::handleIdleEntry() // never get here because entry is
set IN_MEMORY status // already marked for release
The patch adds StoreController::keepInLocalMemory() method to determine
if an entry should be kept in memory for later use. It uses different
checks depending on the configuration: shared memory cache checks are
implemented in MemStore::keepInLocalMemory() and local ones are in
StoreController::keepInLocalMemoryCache(). Both methods use
StoreEntry::memoryCachable() for general checks. Shared memory
cache-specific checks are moved from StoreEntry::memoryCachable() to
MemStore::keepInLocalMemory().
=== modified file 'src/MemStore.cc'
--- src/MemStore.cc 2011-10-28 01:01:41 +0000
+++ src/MemStore.cc 2012-07-05 21:17:28 +0000
@@ -76,40 +76,51 @@ MemStore::stat(StoreEntry &e) const
const int limit = map->entryLimit();
storeAppendPrintf(&e, "Maximum entries: %9d\n", limit);
if (limit > 0) {
storeAppendPrintf(&e, "Current entries: %"PRId64" %.2f%%\n",
currentCount(), (100.0 * currentCount() / limit));
if (limit < 100) { // XXX: otherwise too expensive to count
Ipc::ReadWriteLockStats stats;
map->updateStats(stats);
stats.dump(e);
}
}
}
}
void
MemStore::maintain()
{
}
+bool
+MemStore::keepInLocalMemory(const StoreEntry &e) const {
+ if (!e.memoryCachable())
+ return false;
+
+ Must(e.mem_obj);
+ const int64_t size =
+ max(e.mem_obj->expectedReplySize(), e.mem_obj->endOffset());
+ return willFit(size);
+}
+
uint64_t
MemStore::minSize() const
{
return 0; // XXX: irrelevant, but Store parent forces us to implement this
}
uint64_t
MemStore::maxSize() const
{
return 0; // XXX: make configurable
}
uint64_t
MemStore::currentSize() const
{
return theCurrentSize;
}
uint64_t
MemStore::currentCount() const
@@ -252,41 +263,41 @@ MemStore::copyFromShm(StoreEntry &e, con
}
void
MemStore::considerKeeping(StoreEntry &e)
{
if (!e.memoryCachable()) {
debugs(20, 7, HERE << "Not memory cachable: " << e);
return; // cannot keep due to entry state or properties
}
assert(e.mem_obj);
if (!willFit(e.mem_obj->endOffset())) {
debugs(20, 5, HERE << "No mem-cache space for " << e);
return; // failed to free enough space
}
keep(e); // may still fail
}
bool
-MemStore::willFit(int64_t need)
+MemStore::willFit(const int64_t need) const
{
// TODO: obey configured maximum entry size (with page-based rounding)
return need <= static_cast<int64_t>(Ipc::Mem::PageSize());
}
/// allocates map slot and calls copyToShm to store the entry in shared memory
void
MemStore::keep(StoreEntry &e)
{
if (!map) {
debugs(20, 5, HERE << "No map to mem-cache " << e);
return;
}
sfileno index = 0;
Ipc::StoreMapSlot *slot = map->openForWriting(reinterpret_cast<const cache_key *>(e.key), index);
if (!slot) {
debugs(20, 5, HERE << "No room in mem-cache map to index " << e);
return;
}
=== modified file 'src/MemStore.h'
--- src/MemStore.h 2011-10-14 16:21:48 +0000
+++ src/MemStore.h 2012-07-05 21:12:45 +0000
@@ -26,45 +26,46 @@ public:
/// cache the entry or forget about it until the next considerKeeping call
void considerKeeping(StoreEntry &e);
/* 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 void maintain();
+ virtual bool keepInLocalMemory(const StoreEntry &e) const;
static int64_t EntryLimit();
protected:
- bool willFit(int64_t needed);
+ bool willFit(const int64_t need) 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".
// Why not just use a SwapDir API? That would not help much because Store has
// to check/update memory cache separately from the disk cache. And same API
// would hurt because we can support synchronous get/put, unlike the disks.
#endif /* SQUID_MEMSTORE_H */
=== modified file 'src/Store.h'
--- src/Store.h 2012-01-13 13:49:26 +0000
+++ src/Store.h 2012-07-05 21:24:31 +0000
@@ -334,40 +334,43 @@ public:
/** 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 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) {}
+ /// whether entry should be kept in local memory for later use
+ virtual bool keepInLocalMemory(const StoreEntry &) const { return false; }
+
private:
static RefCount<Store> CurrentRoot;
};
/// \ingroup StoreAPI
typedef RefCount<Store> StorePointer;
/// \ingroup StoreAPI
SQUIDCEXTERN size_t storeEntryInUse();
/// \ingroup StoreAPI
SQUIDCEXTERN const char *storeEntryFlags(const StoreEntry *);
/// \ingroup StoreAPI
extern void storeEntryReplaceObject(StoreEntry *, HttpReply *);
/// \ingroup StoreAPI
SQUIDCEXTERN StoreEntry *storeGetPublic(const char *uri, const HttpRequestMethod& method);
/// \ingroup StoreAPI
=== modified file 'src/SwapDir.h'
--- src/SwapDir.h 2011-10-27 23:14:28 +0000
+++ src/SwapDir.h 2012-07-05 20:54:42 +0000
@@ -69,45 +69,48 @@ public:
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 keepInLocalMemory(const StoreEntry &e) const;
+
/* the number of store dirs being rebuilt. */
static int store_dirs_rebuilding;
private:
void createOneStore(Store &aStore);
+ bool keepInLocalMemoryCache(const StoreEntry &e) const;
StorePointer swapDir; ///< summary view of all disk caches
MemStore *memStore; ///< memory cache
};
/* migrating from the Config based list of swapdirs */
extern void allocate_new_swapdir(SquidConfig::_cacheSwap *);
extern void free_cachedir(SquidConfig::_cacheSwap * swap);
SQUIDCEXTERN OBJH storeDirStats;
SQUIDCEXTERN char *storeDirSwapLogFile(int, const char *);
SQUIDCEXTERN char *storeSwapFullPath(int, char *);
SQUIDCEXTERN char *storeSwapSubSubDir(int, char *);
SQUIDCEXTERN const char *storeSwapPath(int);
SQUIDCEXTERN int storeDirWriteCleanLogs(int reopen);
SQUIDCEXTERN STDIRSELECT *storeDirSelectSwapDir;
SQUIDCEXTERN int storeVerifySwapDirs(void);
SQUIDCEXTERN void storeDirCloseSwapLogs(void);
SQUIDCEXTERN void storeDirCloseTmpSwapLog(int dirn);
SQUIDCEXTERN void storeDirDiskFull(sdirno);
SQUIDCEXTERN void storeDirOpenSwapLogs(void);
=== modified file 'src/store.cc'
--- src/store.cc 2012-01-13 13:49:26 +0000
+++ src/store.cc 2012-07-05 21:25:34 +0000
@@ -1435,47 +1435,44 @@ storeConfigure(void)
store_swap_low = (long) (((float) Store::Root().maxSize() *
(float) Config.Swap.lowWaterMark) / (float) 100);
store_pages_max = Config.memMaxSize / sizeof(mem_node);
}
bool
StoreEntry::memoryCachable() const
{
if (mem_obj == NULL)
return 0;
if (mem_obj->data_hdr.size() == 0)
return 0;
if (mem_obj->inmem_lo != 0)
return 0;
if (!Config.onoff.memory_cache_first && swap_status == SWAPOUT_DONE && refcount == 1)
return 0;
- if (Config.memShared && IamWorkerProcess()) {
- const int64_t expectedSize = mem_obj->expectedReplySize();
- // objects of unknown size are not allowed into memory cache, for now
- if (expectedSize < 0 ||
- expectedSize > static_cast<int64_t>(Config.Store.maxInMemObjSize))
- return 0;
- }
+ const int64_t size =
+ max(mem_obj->expectedReplySize(), mem_obj->endOffset());
+ if (size > static_cast<int64_t>(Config.Store.maxInMemObjSize))
+ return 0;
return 1;
}
int
StoreEntry::checkNegativeHit() const
{
if (!EBIT_TEST(flags, ENTRY_NEGCACHED))
return 0;
if (expires <= squid_curtime)
return 0;
if (store_status != STORE_OK)
return 0;
return 1;
}
/**
@@ -1885,40 +1882,43 @@ StoreEntry::getSerialisedMetaData()
StoreMeta *tlv_list = storeSwapMetaBuild(this);
int swap_hdr_sz;
char *result = storeSwapMetaPack(tlv_list, &swap_hdr_sz);
storeSwapTLVFree(tlv_list);
assert (swap_hdr_sz >= 0);
mem_obj->swap_hdr_sz = (size_t) swap_hdr_sz;
return result;
}
void
StoreEntry::trimMemory(const bool preserveSwappable)
{
/*
* DPW 2007-05-09
* Bug #1943. We must not let go any data for IN_MEMORY
* objects. We have to wait until the mem_status changes.
*/
if (mem_status == IN_MEMORY)
return;
+ if (Store::Root().keepInLocalMemory(*this))
+ return; // keep entry in memory for later use
+
if (!preserveSwappable) {
if (mem_obj->policyLowestOffsetToKeep(0) == 0) {
/* Nothing to do */
return;
}
/*
* Its not swap-able, and we're about to delete a chunk,
* so we must make it PRIVATE. This is tricky/ugly because
* for the most part, we treat swapable == cachable here.
*/
releaseRequest();
mem_obj->trimUnSwappable ();
} else {
mem_obj->trimSwappable ();
}
}
bool
StoreEntry::modifiedSince(HttpRequest * request) const
{
=== modified file 'src/store_dir.cc'
--- src/store_dir.cc 2011-10-14 16:21:48 +0000
+++ src/store_dir.cc 2012-07-05 21:02:36 +0000
@@ -707,40 +707,57 @@ StoreController::dereference(StoreEntry
bool keepInStoreTable = true; // keep if there are no objections
/* Notify the fs that we're not referencing this object any more */
if (e.swap_filen > -1)
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;
// 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);
}
return keepInStoreTable;
}
+bool
+StoreController::keepInLocalMemory(const StoreEntry &e) const
+{
+ return memStore ? memStore->keepInLocalMemory(e) :
+ keepInLocalMemoryCache(e);
+}
+
+// whether entry should be kept in local memory cache
+bool
+StoreController::keepInLocalMemoryCache(const StoreEntry &e) const
+{
+ Must(!memStore); // Should(), really
+ return e.memoryCachable() && // entry is in good shape and
+ // the local memory cache is not overflowing
+ (mem_node::InUseCount() <= store_pages_max);
+}
+
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;
}
}
// TODO: this disk iteration is misplaced; move to StoreHashIndex when
// the global store_table is no longer used for in-transit objects.
if (const int cacheDirs = Config.cacheSwap.n_configured) {
@@ -763,45 +780,42 @@ StoreController::get(const cache_key *ke
}
debugs(20, 4, HERE << "none of " << Config.cacheSwap.n_configured <<
" cache_dirs have " << storeKeyText(key));
return NULL;
}
void
StoreController::get(String const key, STOREGETCLIENT aCallback, void *aCallbackData)
{
fatal("not implemented");
}
void
StoreController::handleIdleEntry(StoreEntry &e)
{
bool keepInLocalMemory = false;
if (memStore) {
memStore->considerKeeping(e);
// leave keepInLocalMemory false; memStore maintains its own cache
- } else {
- keepInLocalMemory = e.memoryCachable() && // entry is in good shape and
- // the local memory cache is not overflowing
- (mem_node::InUseCount() <= store_pages_max);
- }
+ } else
+ keepInLocalMemory = keepInLocalMemoryCache(e);
// An idle, unlocked entry that belongs to a SwapDir which controls
// its own index, should not stay in the global store_table.
if (!dereference(e)) {
debugs(20, 5, HERE << "destroying unlocked entry: " << &e << ' ' << e);
destroyStoreEntry(static_cast<hash_link*>(&e));
return;
}
// 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()
{
