Hi all.
Local memory caching does not work in recent Squid trunk. The patch
attempts to fix that.
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 StoreEntry::keepInLocalMemory() method to check if an
entry should be stored in local memory cache. If it is true, do not
release entry in trimMemory().
Regards,
Dmitry
Do not release entries that should be kept in local memory cache.
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 StoreEntry::keepInLocalMemory() method to check if an
entry should be stored in local memory cache. If it is true, do not
release entry in trimMemory().
=== modified file 'src/Store.h'
--- src/Store.h 2012-01-13 13:49:26 +0000
+++ src/Store.h 2012-07-05 01:23:03 +0000
@@ -101,40 +101,42 @@ public:
void makePrivate();
void setPublicKey();
void setPrivateKey();
void expireNow();
void releaseRequest();
void negativeCache();
void cacheNegatively(); /** \todo argh, why both? */
void invokeHandlers();
void purgeMem();
void cacheInMemory(); ///< start or continue storing in memory cache
void swapOut();
/// whether we are in the process of writing this entry to disk
bool swappingOut() const { return swap_status == SWAPOUT_WRITING; }
void swapOutFileClose(int how);
const char *url() const;
int checkCachable();
int checkNegativeHit() const;
int locked() const;
int validToSend() const;
bool memoryCachable() const; ///< may be cached in memory
+ /// may be cached in memory and local memory cache is not overflowing
+ bool keepInLocalMemory() const;
void createMemObject(const char *, const char *);
void hideMemObject(); ///< no mem_obj for callers until createMemObject
void dump(int debug_lvl) const;
void hashDelete();
void hashInsert(const cache_key *);
void registerAbort(STABH * cb, void *);
void reset();
void setMemStatus(mem_status_t);
void timestampsSet();
void unregisterAbort();
void destroyMemObject();
int checkTooSmall();
void delayAwareRead(const Comm::ConnectionPointer &conn, char *buf, int len, AsyncCall::Pointer callback);
void setNoDelay (bool const);
bool modifiedSince(HttpRequest * request) const;
/// has ETag matching at least one of the If-Match etags
bool hasIfMatchEtag(const HttpRequest &request) const;
/// has ETag matching at least one of the If-None-Match etags
=== modified file 'src/store.cc'
--- src/store.cc 2012-01-13 13:49:26 +0000
+++ src/store.cc 2012-07-05 00:30:45 +0000
@@ -1446,40 +1446,46 @@ StoreEntry::memoryCachable() const
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;
}
return 1;
}
+bool
+StoreEntry::keepInLocalMemory() const
+{
+ return memoryCachable() && (mem_node::InUseCount() <= store_pages_max);
+}
+
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;
}
/**
* Set object for negative caching.
* Preserves any expiry information given by the server.
* In absence of proper expiry info it will set to expire immediately,
* or with HTTP-violations enabled the configured negative-TTL is observed
@@ -1882,41 +1888,41 @@ StoreEntry::startWriting()
char const *
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)
+ if (mem_status == IN_MEMORY || keepInLocalMemory())
return;
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
=== modified file 'src/store_dir.cc'
--- src/store_dir.cc 2011-10-14 16:21:48 +0000
+++ src/store_dir.cc 2012-07-05 00:29:14 +0000
@@ -763,45 +763,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 = e.keepInLocalMemory();
// 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()
{
