Hello,
The attached patch contains three fixes related to Vary caching.
These fixes solve some but not all Vary-related problems. The fixes are
extracted from the unofficial bag5s branch on Launchpad but may apply to
trunk and/or v3.[34] as well. Sorry, I did not have the time to check
that and to update related bug reports yet (and I may not be able to do
it soon enough, so I wanted to post these now).
The comments quoted below (extracted from the branch commit log) explain
the details.
HTH,
Alex.
> revno: 12743
> branch nick: bag5s
> Refuse to cache Vary-controlled objects in shared memory (for now).
>
> Shared memory cache code incorrectly assumed that it just needs to keep
> the response and the STORE_META_STD entry details (because that is what
> non-shared memory cache seemed to keep). However, non-shared memory
> cache also keeps MemObject::vary_headers and possibly other meta information
> that cannot be recovered from STORE_META_STD. Thus, we need to store and
> load all serialized meta objects in shared memory, just like the disk
> cache does.
>
> Until the above is implemented, we must disable Vary caching in shared
> memory
> to avoid "Vary object loop" errors and possibly other problems.
> revno: 12742
> branch nick: bag5s
> Log failed (due to "Vary object loop" or "URL mismatch") hits as TCP_MISSes.
> revno: 12741
> branch nick: bag5s
> Do not start storing the vary marker object until its key becomes public
> or Squid will mark it as "impossible to cache" and will never cache it.
Various fixes making Vary caching work better.
More work is needed to re-enable shared memory caching of Vary responses.
bag5s r12741: Do not start storing the vary marker object until its key becomes public.
bag5s r12742: Log failed (due to "Vary object loop" or "URL mismatch") hits as TCP_MISSes.
bag5s r12743: Refuse to cache Vary-controlled objects in shared memory (for now).
=== modified file 'src/MemStore.cc'
--- src/MemStore.cc 2013-01-20 18:54:42 +0000
+++ src/MemStore.cc 2013-11-08 18:42:40 +0000
@@ -352,40 +352,46 @@ MemStore::considerKeeping(StoreEntry &e)
}
assert(e.mem_obj);
const int64_t loadedSize = e.mem_obj->endOffset();
const int64_t expectedSize = e.mem_obj->expectedReplySize();
// objects of unknown size are not allowed into memory cache, for now
if (expectedSize < 0) {
debugs(20, 5, HERE << "Unknown expected size: " << e);
return;
}
// since we copy everything at once, we can only keep fully loaded entries
if (loadedSize != expectedSize) {
debugs(20, 7, HERE << "partially loaded: " << loadedSize << " != " <<
expectedSize);
return;
}
+ if (e.mem_obj->vary_headers) {
+ // XXX: We must store/load SerialisedMetaData to cache Vary in RAM
+ debugs(20, 5, "Vary not yet supported: " << e.mem_obj->vary_headers);
+ return;
+ }
+
keep(e); // may still fail
}
/// locks map anchor 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::StoreMapAnchor *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;
}
try {
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2013-03-21 21:06:48 +0000
+++ src/client_side_reply.cc 2013-11-08 18:04:33 +0000
@@ -482,71 +482,73 @@ clientReplyContext::cacheHit(StoreIOBuff
/* the store couldn't get enough data from the file for us to id the
* object
*/
/* treat as a miss */
http->logType = LOG_TCP_MISS;
processMiss();
return;
}
assert(!EBIT_TEST(e->flags, ENTRY_ABORTED));
/* update size of the request */
reqsize = result.length + reqofs;
/*
* Got the headers, now grok them
*/
assert(http->logType == LOG_TCP_HIT);
if (strcmp(e->mem_obj->url, http->request->storeId()) != 0) {
debugs(33, DBG_IMPORTANT, "clientProcessHit: URL mismatch, '" << e->mem_obj->url << "' != '" << http->request->storeId() << "'");
+ http->logType = LOG_TCP_MISS; // we lack a more precise LOG_*_MISS code
processMiss();
return;
}
switch (varyEvaluateMatch(e, r)) {
case VARY_NONE:
/* No variance detected. Continue as normal */
break;
case VARY_MATCH:
/* This is the correct entity for this request. Continue */
debugs(88, 2, "clientProcessHit: Vary MATCH!");
break;
case VARY_OTHER:
/* This is not the correct entity for this request. We need
* to requery the cache.
*/
removeClientStoreReference(&sc, http);
e = NULL;
/* Note: varyEvalyateMatch updates the request with vary information
* so we only get here once. (it also takes care of cancelling loops)
*/
debugs(88, 2, "clientProcessHit: Vary detected!");
clientGetMoreData(ourNode, http);
return;
case VARY_CANCEL:
/* varyEvaluateMatch found a object loop. Process as miss */
debugs(88, DBG_IMPORTANT, "clientProcessHit: Vary object loop!");
+ http->logType = LOG_TCP_MISS; // we lack a more precise LOG_*_MISS code
processMiss();
return;
}
if (r->method == Http::METHOD_PURGE) {
removeClientStoreReference(&sc, http);
e = NULL;
purgeRequest();
return;
}
if (e->checkNegativeHit()
&& !r->flags.noCacheHack()
) {
http->logType = LOG_TCP_NEGATIVE_HIT;
sendMoreData(result);
} else if (!http->flags.internal && refreshCheckHTTP(e, r)) {
debugs(88, 5, "clientCacheHit: in refreshCheck() block");
/*
* We hold a stale copy; it needs to be validated
=== modified file 'src/store.cc'
--- src/store.cc 2013-03-21 21:06:48 +0000
+++ src/store.cc 2013-11-08 17:50:49 +0000
@@ -772,46 +772,48 @@ StoreEntry::setPublicKey()
HttpReply *rep = new HttpReply;
rep->setHeaders(Http::scOkay, "Internal marker object", "x-squid-internal/vary", -1, -1, squid_curtime + 100000);
vary = mem_obj->getReply()->header.getList(HDR_VARY);
if (vary.size()) {
/* Again, we own this structure layout */
rep->header.putStr(HDR_VARY, vary.termedBuf());
vary.clean();
}
#if X_ACCELERATOR_VARY
vary = mem_obj->getReply()->header.getList(HDR_X_ACCELERATOR_VARY);
if (vary.defined()) {
/* Again, we own this structure layout */
rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf());
vary.clean();
}
#endif
- pe->replaceHttpReply(rep);
+ pe->replaceHttpReply(rep, false); // no write until key is public
pe->timestampsSet();
pe->makePublic();
+ pe->startWriting(); // after makePublic()
+
pe->complete();
pe->unlock();
}
newkey = storeKeyPublicByRequest(mem_obj->request);
} else
newkey = storeKeyPublic(mem_obj->url, mem_obj->method);
if ((e2 = (StoreEntry *) hash_lookup(store_table, newkey))) {
debugs(20, 3, "StoreEntry::setPublicKey: Making old '" << mem_obj->url << "' private.");
e2->setPrivateKey();
e2->release();
if (mem_obj->request)
newkey = storeKeyPublicByRequest(mem_obj->request);
else
newkey = storeKeyPublic(mem_obj->url, mem_obj->method);
}
