------------------------------------------------------------ revno: 14088 revision-id: squid3@treenet.co.nz-20160923152842-wstun879sjafjg2p parent: squid3@treenet.co.nz-20160923124044-i1road76n1108syf fixes bugs: http://bugs.squid-cache.org/show_bug.cgi?id=4311 http://bugs.squid-cache.org/show_bug.cgi?id=2833 http://bugs.squid-cache.org/show_bug.cgi?id=4471 author: Eduard Bagdasaryan committer: Amos Jeffries branch nick: 3.5 timestamp: Sat 2016-09-24 03:28:42 +1200 message: Bug 2833: Collapse internal revalidation requests (SMP-unaware caches) ... also address Bug 4311 and partially address Bug 2833 and Bug 4471. Extend collapsed_forwarding functionality to internal revalidation requests. This implementation does not support Vary-controlled cache objects and is limited to SMP-unaware caching environments, where each Squid worker knows nothing about requests and caches handled by other workers. However, it also lays critical groundwork for future SMP-aware collapsed revalidation support. Prior to these changes, multiple concurrent HTTP requests for the same stale cached object always resulted in multiple internal revalidation requests sent by Squid to the origin server. Those internal requests were likely to result in multiple competing Squid cache updates, causing cache misses and/or more internal revalidation requests, negating collapsed forwarding savings. Internal cache revalidation requests are collapsed if and only if collapsed_forwarding is enabled. There is no option to control just revalidation collapsing because there is no known use case for it. * Public revalidation keys Each Store entry has a unique key. Keys are used to find entries in the Store (both already cached/swapped_out entries and not). Public keys are normally tied to the request method and target URI. Same request properties normally lead to the same public key, making cache hits possible. If we were to calculate a public key for an internal revalidation request, it would have been the same as the public key of the stale cache entry being revalidated. Adding a revalidation response to Store would have purged that being-revalidated cached entry, even if the revalidation response itself was not cachable. To avoid purging being-revalidated cached entries, the old code used private keys for internal revalidation requests. Private keys are always unique and cannot accidentally purge a public entry. On the other hand, for concurrent [revalidation] requests to find the store entry to collapse on, that store entry has to have a public key! We resolved this conflict by adding "scope" to public keys: * Regular/old public keys have default empty scope (that does not affect key generation). The code not dealing with collapsed revalidation continues to work as before. All entries stored in caches continue to have the same keys (with an empty scope). * When collapsed forwarding is enabled, collapsable internal revalidation requests get public keys with a "revalidation" scope (that is fed to the MD5 hash when the key is generated). Such a revalidation request can find a matching store entry created by another revalidation request (and collapse on it), but cannot clash with the entry being revalidated (because that entry key is using a different [empty] scope). This change not only enables collapsing of internal revalidation requests within one worker, but opens the way for SMP-aware workers to share information about collapsed revalidation requests, similar to how those workers already share information about being-swapped-out cache entries. After receiving the revalidation response, each collapsed revalidation request may call updateOnNotModified() to update the stale entry [with the same revalidation response!]. Concurrent entry updates would have wasted many resources, especially for disk-cached entries that support header updates, and may have purged being-revalidated entries due to locking conflicts among updating transactions. To minimize these problems, we adjusted header and entry metadata updating logic to skip the update if nothing have changed since the last update. Also fixed Bug 4311: Collapsed forwarding deadlocks for SMP Squids using SMP-unaware caches. Collapsed transactions stalled without getting a response because they were waiting for the shared "transients" table updates. The table was created by Store::Controller but then abandoned (not updated) by SMP-unaware caches. Now, the transients table is not created at all unless SMP-aware caches are present. This fix should also address complaints about shared memory being used for Squid instances without SMP-aware caches. A combination of SMP-aware and SMP-unaware caches is still not supported and is likely to stall collapsed transactions if they are enabled. Note that, by default, the memory cache is SMP-aware in SMP environments. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20160923152842-wstun879sjafjg2p # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # testament_sha1: 9bec994af031236daece9809ab5b219a0e7edde5 # timestamp: 2016-09-23 15:51:02 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # base_revision_id: squid3@treenet.co.nz-20160923124044-\ # i1road76n1108syf # # Begin patch === modified file 'src/HttpHeader.cc' --- src/HttpHeader.cc 2016-01-01 00:14:27 +0000 +++ src/HttpHeader.cc 2016-09-23 15:28:42 +0000 @@ -450,7 +450,7 @@ HttpHeader::HttpHeader(const HttpHeader &other): owner(other.owner), len(other.len), conflictingContentLength_(false) { httpHeaderMaskInit(&mask, 0); - update(&other, NULL); // will update the mask as well + update(&other); // will update the mask as well } HttpHeader::~HttpHeader() @@ -465,7 +465,7 @@ // we do not really care, but the caller probably does assert(owner == other.owner); clean(); - update(&other, NULL); // will update the mask as well + update(&other); // will update the mask as well len = other.len; conflictingContentLength_ = other.conflictingContentLength_; } @@ -535,26 +535,71 @@ } } +/// check whether the fresh header has any new/changed updatable fields +bool +HttpHeader::needUpdate(HttpHeader const *fresh) const +{ + for (unsigned int i = 0; i < fresh->entries.size(); ++i) { + const HttpHeaderEntry *e = fresh->entries[i]; + if (!e || skipUpdateHeader(e->id)) + continue; + String value; + const char *name = e->name.termedBuf(); + if (!getByNameIfPresent(name, value) || + (value != fresh->getByName(name))) + return true; + } + return false; +} + /* use fresh entries to replace old ones */ void httpHeaderUpdate(HttpHeader * old, const HttpHeader * fresh, const HttpHeaderMask * denied_mask) { assert (old); - old->update (fresh, denied_mask); + old->update(fresh); } void -HttpHeader::update (HttpHeader const *fresh, HttpHeaderMask const *denied_mask) +HttpHeader::updateWarnings() { - const HttpHeaderEntry *e; + int count = 0; HttpHeaderPos pos = HttpHeaderInitPos; + + // RFC 7234, section 4.3.4: delete 1xx warnings and retain 2xx warnings + while (HttpHeaderEntry *e = getEntry(&pos)) { + if (e->id == HDR_WARNING && (e->getInt()/100 == 1) ) + delAt(pos, count); + } +} + +bool +HttpHeader::skipUpdateHeader(const http_hdr_type id) const +{ + // RFC 7234, section 4.3.4: use fields other from Warning for update + return id == HDR_WARNING; +} + +bool +HttpHeader::update(HttpHeader const *fresh) +{ assert(fresh); assert(this != fresh); + // Optimization: Finding whether a header field changed is expensive + // and probably not worth it except for collapsed revalidation needs. + if (Config.onoff.collapsed_forwarding && !needUpdate(fresh)) + return false; + + updateWarnings(); + + const HttpHeaderEntry *e; + HttpHeaderPos pos = HttpHeaderInitPos; + while ((e = fresh->getEntry(&pos))) { /* deny bad guys (ok to check for HDR_OTHER) here */ - if (denied_mask && CBIT_TEST(*denied_mask, e->id)) + if (skipUpdateHeader(e->id)) continue; if (e->id != HDR_OTHER) @@ -567,13 +612,14 @@ while ((e = fresh->getEntry(&pos))) { /* deny bad guys (ok to check for HDR_OTHER) here */ - if (denied_mask && CBIT_TEST(*denied_mask, e->id)) + if (skipUpdateHeader(e->id)) continue; debugs(55, 7, "Updating header '" << HeadersAttrs[e->id].name << "' in cached entry"); addEntry(e->clone()); } + return true; } /* just handy in parsing: resets and returns false */ @@ -1720,7 +1766,6 @@ HttpHeaderEntry::getInt() const { assert_eid (id); - assert (Headers[id].type == ftInt); int val = -1; int ok = httpHeaderParseInt(value.termedBuf(), &val); httpHeaderNoteParsedEntry(id, value, !ok); @@ -1734,7 +1779,6 @@ HttpHeaderEntry::getInt64() const { assert_eid (id); - assert (Headers[id].type == ftInt64); int64_t val = -1; int ok = httpHeaderParseOffset(value.termedBuf(), &val); httpHeaderNoteParsedEntry(id, value, !ok); === modified file 'src/HttpHeader.h' --- src/HttpHeader.h 2016-01-01 00:14:27 +0000 +++ src/HttpHeader.h 2016-09-23 15:28:42 +0000 @@ -217,7 +217,7 @@ /* Interface functions */ void clean(); void append(const HttpHeader * src); - void update (HttpHeader const *fresh, HttpHeaderMask const *denied_mask); + bool update(HttpHeader const *fresh); void compact(); int reset(); int parse(const char *header_start, const char *header_end); @@ -278,6 +278,9 @@ protected: /** \deprecated Public access replaced by removeHopByHopEntries() */ void removeConnectionHeaderEntries(); + bool needUpdate(const HttpHeader *fresh) const; + bool skipUpdateHeader(const http_hdr_type id) const; + void updateWarnings(); private: HttpHeaderEntry *findLastEntry(http_hdr_type id) const; === modified file 'src/HttpReply.cc' --- src/HttpReply.cc 2016-03-23 15:36:45 +0000 +++ src/HttpReply.cc 2016-09-23 15:28:42 +0000 @@ -24,39 +24,6 @@ #include "Store.h" #include "StrList.h" -/* local constants */ - -/* If we receive a 304 from the origin during a cache revalidation, we must - * update the headers of the existing entry. Specifically, we need to update all - * end-to-end headers and not any hop-by-hop headers (rfc2616 13.5.3). - * - * This is not the whole story though: since it is possible for a faulty/malicious - * origin server to set headers it should not in a 304, we must explicitly ignore - * these too. Specifically all entity-headers except those permitted in a 304 - * (rfc2616 10.3.5) must be ignored. - * - * The list of headers we don't update is made up of: - * all hop-by-hop headers - * all entity-headers except Expires and Content-Location - */ -static HttpHeaderMask Denied304HeadersMask; -static http_hdr_type Denied304HeadersArr[] = { - // hop-by-hop headers - HDR_CONNECTION, HDR_KEEP_ALIVE, HDR_PROXY_AUTHENTICATE, HDR_PROXY_AUTHORIZATION, - HDR_TE, HDR_TRAILER, HDR_TRANSFER_ENCODING, HDR_UPGRADE, - // entity headers - HDR_ALLOW, HDR_CONTENT_ENCODING, HDR_CONTENT_LANGUAGE, HDR_CONTENT_LENGTH, - HDR_CONTENT_MD5, HDR_CONTENT_RANGE, HDR_CONTENT_TYPE, HDR_LAST_MODIFIED -}; - -/* module initialization */ -void -httpReplyInitModule(void) -{ - assert(Http::scNone == 0); // HttpReply::parse() interface assumes that - httpHeaderMaskInit(&Denied304HeadersMask, 0); - httpHeaderCalcMask(&Denied304HeadersMask, Denied304HeadersArr, countof(Denied304HeadersArr)); -} HttpReply::HttpReply() : HttpMsg(hoReply), date (0), last_modified (0), expires (0), surrogate_control (NULL), content_range (NULL), keep_alive (0), @@ -271,20 +238,23 @@ return 1; } -void +bool HttpReply::updateOnNotModified(HttpReply const * freshRep) { assert(freshRep); + /* update raw headers */ + if (!header.update(&freshRep->header)) + return false; + /* clean cache */ hdrCacheClean(); - /* update raw headers */ - header.update(&freshRep->header, - (const HttpHeaderMask *) &Denied304HeadersMask); header.compact(); /* init cache */ hdrCacheInit(); + + return true; } /* internal routines */ === modified file 'src/HttpReply.h' --- src/HttpReply.h 2016-01-01 00:14:27 +0000 +++ src/HttpReply.h 2016-09-23 15:28:42 +0000 @@ -72,7 +72,7 @@ virtual bool inheritProperties(const HttpMsg *aMsg); - void updateOnNotModified(HttpReply const *other); + bool updateOnNotModified(HttpReply const *other); /** set commonly used info with one call */ void setHeaders(Http::StatusCode status, === modified file 'src/MemStore.h' --- src/MemStore.h 2016-01-01 00:14:27 +0000 +++ src/MemStore.h 2016-09-23 15:28:42 +0000 @@ -63,6 +63,7 @@ virtual void maintain(); virtual bool anchorCollapsed(StoreEntry &collapsed, bool &inSync); virtual bool updateCollapsed(StoreEntry &collapsed); + virtual bool smpAware() const { return true; } static int64_t EntryLimit(); === modified file 'src/Store.h' --- src/Store.h 2016-01-01 00:14:27 +0000 +++ src/Store.h 2016-09-23 15:28:42 +0000 @@ -24,6 +24,7 @@ #include "Range.h" #include "RemovalPolicy.h" #include "StoreIOBuffer.h" +#include "store_key_md5.h" #include "StoreStats.h" #if USE_SQUID_ESI @@ -93,9 +94,13 @@ void abort(); void unlink(); - void makePublic(); + void makePublic(const KeyScope keyScope = ksDefault); void makePrivate(); - void setPublicKey(); + void setPublicKey(const KeyScope keyScope = ksDefault); + /// Resets existing public key to a public key with default scope, + /// releasing the old default-scope entry (if any). + /// Does nothing if the existing public key already has default scope. + void clearPublicKeyScope(); void setPrivateKey(); void expireNow(); void releaseRequest(); @@ -130,7 +135,7 @@ void registerAbort(STABH * cb, void *); void reset(); void setMemStatus(mem_status_t); - void timestampsSet(); + bool timestampsSet(); void unregisterAbort(); void destroyMemObject(); int checkTooSmall(); @@ -228,6 +233,9 @@ private: bool checkTooBig() const; + void forcePublicKey(const cache_key *newkey); + void adjustVary(); + const cache_key *calcPublicKey(const KeyScope keyScope); static MemAllocator *pool; @@ -430,6 +438,9 @@ /// update a local collapsed entry with fresh info from this cache (if any) virtual bool updateCollapsed(StoreEntry &collapsed) { return false; } + /// whether this storage is capable of serving multiple workers + virtual bool smpAware() const = 0; + private: static RefCount CurrentRoot; }; @@ -450,10 +461,10 @@ StoreEntry *storeGetPublic(const char *uri, const HttpRequestMethod& method); /// \ingroup StoreAPI -StoreEntry *storeGetPublicByRequest(HttpRequest * request); +StoreEntry *storeGetPublicByRequest(HttpRequest * request, const KeyScope keyScope = ksDefault); /// \ingroup StoreAPI -StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method); +StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method, const KeyScope keyScope = ksDefault); /// \ingroup StoreAPI /// Like storeCreatePureEntry(), but also locks the entry and sets entry key. === modified file 'src/StoreHashIndex.h' --- src/StoreHashIndex.h 2016-01-01 00:14:27 +0000 +++ src/StoreHashIndex.h 2016-09-23 15:28:42 +0000 @@ -59,6 +59,8 @@ virtual StoreSearch *search(String const url, HttpRequest *); + virtual bool smpAware() const; + private: /* migration logic */ StorePointer store(int const x) const; === modified file 'src/SwapDir.h' --- src/SwapDir.h 2016-01-01 00:14:27 +0000 +++ src/SwapDir.h 2016-09-23 15:28:42 +0000 @@ -51,6 +51,7 @@ virtual void memoryDisconnect(StoreEntry &e); virtual void allowCollapsing(StoreEntry *e, const RequestFlags &reqFlags, const HttpRequestMethod &reqMethod); virtual void syncCollapsed(const sfileno xitIndex); + virtual bool smpAware() const; virtual void init(); @@ -158,6 +159,7 @@ virtual void getStats(StoreInfoStats &stats) const; virtual void stat (StoreEntry &anEntry) const; virtual StoreSearch *search(String const url, HttpRequest *) = 0; + virtual bool smpAware() const { return false; } /* migrated from store_dir.cc */ bool objectSizeIsAcceptable(int64_t objsize) const; === modified file 'src/Transients.cc' --- src/Transients.cc 2016-01-01 00:14:27 +0000 +++ src/Transients.cc 2016-09-23 15:28:42 +0000 @@ -197,7 +197,8 @@ e->mem_obj->xitTable.io = MemObject::ioReading; e->mem_obj->xitTable.index = index; - e->setPublicKey(); + // TODO: Support collapsed revalidation for SMP-aware caches + e->setPublicKey(ksDefault); assert(e->key); // How do we know its SMP- and not just locally-collapsed? A worker gets === modified file 'src/Transients.h' --- src/Transients.h 2016-01-01 00:14:27 +0000 +++ src/Transients.h 2016-09-23 15:28:42 +0000 @@ -72,6 +72,7 @@ virtual bool dereference(StoreEntry &, bool); virtual void markForUnlink(StoreEntry &e); virtual void maintain(); + virtual bool smpAware() const { return true; } static int64_t EntryLimit(); === modified file 'src/adaptation/History.cc' --- src/adaptation/History.cc 2016-01-01 00:14:27 +0000 +++ src/adaptation/History.cc 2016-09-23 15:28:42 +0000 @@ -150,9 +150,9 @@ void Adaptation::History::recordMeta(const HttpHeader *lm) { lastMeta.clean(); - lastMeta.update(lm, NULL); + lastMeta.update(lm); - allMeta.update(lm, NULL); + allMeta.update(lm); allMeta.compact(); } === modified file 'src/cf.data.pre' --- src/cf.data.pre 2016-09-16 11:53:28 +0000 +++ src/cf.data.pre 2016-09-23 15:28:42 +0000 @@ -6105,17 +6105,29 @@ LOC: Config.onoff.collapsed_forwarding DEFAULT: off DOC_START - This option controls whether Squid is allowed to merge multiple - potentially cachable requests for the same URI before Squid knows - whether the response is going to be cachable. - - This feature is disabled by default: Enabling collapsed forwarding - needlessly delays forwarding requests that look cachable (when they are - collapsed) but then need to be forwarded individually anyway because - they end up being for uncachable content. However, in some cases, such - as accelleration of highly cachable content with periodic or groupped - expiration times, the gains from collapsing [large volumes of - simultenous refresh requests] outweigh losses from such delays. + When enabled, instead of forwarding each concurrent request for + the same URL, Squid just sends the first of them. The other, so + called "collapsed" requests, wait for the response to the first + request and, if it happens to be cachable, use that response. + Here, "concurrent requests" means "received after the first + request headers were parsed and before the corresponding response + headers were parsed". + + This feature is disabled by default: enabling collapsed + forwarding needlessly delays forwarding requests that look + cachable (when they are collapsed) but then need to be forwarded + individually anyway because they end up being for uncachable + content. However, in some cases, such as acceleration of highly + cachable content with periodic or grouped expiration times, the + gains from collapsing [large volumes of simultaneous refresh + requests] outweigh losses from such delays. + + Squid collapses two kinds of requests: regular client requests + received on one of the listening ports and internal "cache + revalidation" requests which are triggered by those regular + requests hitting a stale cached object. Revalidation collapsing + is currently disabled for Squid instances containing SMP-aware + disk or memory caches and for Vary-controlled cached objects. DOC_END COMMENT_START === modified file 'src/client_side_reply.cc' --- src/client_side_reply.cc 2016-08-17 05:48:29 +0000 +++ src/client_side_reply.cc 2016-09-23 15:28:42 +0000 @@ -72,7 +72,8 @@ HTTPMSGUNLOCK(reply); } -clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : http (cbdataReference(clientContext)), old_entry (NULL), old_sc(NULL), deleting(false) +clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : http (cbdataReference(clientContext)), old_entry (NULL), old_sc(NULL), deleting(false), +collapsedRevalidation(crNone) {} /** Create an error in the store awaiting the client side to read it. @@ -245,7 +246,6 @@ clientReplyContext::processExpired() { const char *url = storeId(); - StoreEntry *entry = NULL; debugs(88, 3, "clientReplyContext::processExpired: '" << http->uri << "'"); assert(http->storeEntry()->lastmod >= 0); /* @@ -267,9 +267,36 @@ #endif /* Prepare to make a new temporary request */ saveState(); - entry = storeCreateEntry(url, - http->log_uri, http->request->flags, http->request->method); - /* NOTE, don't call StoreEntry->lock(), storeCreateEntry() does it */ + + // TODO: support collapsed revalidation of Vary-controlled entries + const bool collapsingAllowed = Config.onoff.collapsed_forwarding && + !Store::Root().smpAware() && + http->request->vary_headers.isEmpty(); + + StoreEntry *entry = nullptr; + if (collapsingAllowed) { + if ((entry = storeGetPublicByRequest(http->request, ksRevalidation))) + entry->lock("clientReplyContext::processExpired#alreadyRevalidating"); + } + + if (entry) { + debugs(88, 5, "collapsed on existing revalidation entry: " << *entry); + collapsedRevalidation = crSlave; + } else { + entry = storeCreateEntry(url, + http->log_uri, http->request->flags, http->request->method); + /* NOTE, don't call StoreEntry->lock(), storeCreateEntry() does it */ + + if (collapsingAllowed) { + debugs(88, 5, "allow other revalidation requests to collapse on " << *entry); + Store::Root().allowCollapsing(entry, http->request->flags, + http->request->method); + collapsedRevalidation = crInitiator; + } else { + collapsedRevalidation = crNone; + } + } + sc = storeClientListAdd(entry, this); #if USE_DELAY_POOLS /* delay_id is already set on original store client */ @@ -289,12 +316,14 @@ assert(http->out.offset == 0); assert(http->request->clientConnectionManager == http->getConn()); - /* - * A refcounted pointer so that FwdState stays around as long as - * this clientReplyContext does - */ - Comm::ConnectionPointer conn = http->getConn() != NULL ? http->getConn()->clientConnection : NULL; - FwdState::Start(conn, http->storeEntry(), http->request, http->al); + if (collapsedRevalidation != crSlave) { + /* + * A refcounted pointer so that FwdState stays around as long as + * this clientReplyContext does + */ + Comm::ConnectionPointer conn = http->getConn() != NULL ? http->getConn()->clientConnection : NULL; + FwdState::Start(conn, http->storeEntry(), http->request, http->al); + } /* Register with storage manager to receive updates when data comes in. */ @@ -408,6 +437,10 @@ // forward response from origin http->logType = LOG_TCP_REFRESH_MODIFIED; debugs(88, 3, "handleIMSReply: origin replied " << status << ", replacing existing entry and forwarding to client"); + + if (collapsedRevalidation) + http->storeEntry()->clearPublicKeyScope(); + sendClientUpstreamResponse(); } === modified file 'src/client_side_reply.h' --- src/client_side_reply.h 2016-01-01 00:14:27 +0000 +++ src/client_side_reply.h 2016-09-23 15:28:42 +0000 @@ -133,6 +133,14 @@ store_client *old_sc; /* ... for entry to be validated */ bool deleting; + typedef enum { + crNone = 0, ///< collapsed revalidation is not allowed for this context + crInitiator, ///< we initiated collapsed revalidation request + crSlave ///< we collapsed on the existing revalidation request + } CollapsedRevalidation; + + CollapsedRevalidation collapsedRevalidation; + CBDATA_CLASS2(clientReplyContext); }; === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2016-08-17 02:58:28 +0000 +++ src/client_side_request.cc 2016-09-23 15:28:42 +0000 @@ -336,7 +336,7 @@ * correctness. */ if (header) - request->header.update(header, NULL); + request->header.update(header); http->log_uri = xstrdup(urlCanonicalClean(request)); === modified file 'src/esi/Include.cc' --- src/esi/Include.cc 2016-01-01 00:14:27 +0000 +++ src/esi/Include.cc 2016-09-23 15:28:42 +0000 @@ -279,7 +279,7 @@ void ESIInclude::prepareRequestHeaders(HttpHeader &tempheaders, ESIVarState *vars) { - tempheaders.update (&vars->header(), NULL); + tempheaders.update (&vars->header()); tempheaders.removeHopByHopEntries(); } === modified file 'src/fs/rock/RockSwapDir.h' --- src/fs/rock/RockSwapDir.h 2016-01-01 00:14:27 +0000 +++ src/fs/rock/RockSwapDir.h 2016-09-23 15:28:42 +0000 @@ -48,6 +48,7 @@ virtual void swappedOut(const StoreEntry &e); virtual void create(); virtual void parse(int index, char *path); + virtual bool smpAware() const { return true; } // temporary path to the shared memory map of first slots of cached entries SBuf inodeMapPath() const; === modified file 'src/fs/ufs/UFSSwapDir.h' --- src/fs/ufs/UFSSwapDir.h 2016-09-23 12:40:44 +0000 +++ src/fs/ufs/UFSSwapDir.h 2016-09-23 15:28:42 +0000 @@ -90,6 +90,7 @@ virtual void swappedOut(const StoreEntry &e); virtual uint64_t currentSize() const { return cur_size; } virtual uint64_t currentCount() const { return n_disk_objects; } + virtual bool smpAware() const { return false; } void unlinkFile(sfileno f); // move down when unlink is a virtual method === modified file 'src/main.cc' --- src/main.cc 2016-03-02 09:12:10 +0000 +++ src/main.cc 2016-09-23 15:28:42 +0000 @@ -1069,8 +1069,6 @@ httpHeaderInitModule(); /* must go before any header processing (e.g. the one in errorInitialize) */ - httpReplyInitModule(); /* must go before accepting replies */ - errorInitialize(); accessLogInit(); === modified file 'src/store.cc' --- src/store.cc 2016-09-07 15:58:59 +0000 +++ src/store.cc 2016-09-23 15:28:42 +0000 @@ -162,12 +162,12 @@ } void -StoreEntry::makePublic() +StoreEntry::makePublic(const KeyScope scope) { /* This object can be cached for a long time */ if (!EBIT_TEST(flags, RELEASE_REQUEST)) - setPublicKey(); + setPublicKey(scope); } void @@ -585,19 +585,19 @@ } StoreEntry * -storeGetPublicByRequestMethod(HttpRequest * req, const HttpRequestMethod& method) +storeGetPublicByRequestMethod(HttpRequest * req, const HttpRequestMethod& method, const KeyScope keyScope) { - return Store::Root().get(storeKeyPublicByRequestMethod(req, method)); + return Store::Root().get(storeKeyPublicByRequestMethod(req, method, keyScope)); } StoreEntry * -storeGetPublicByRequest(HttpRequest * req) +storeGetPublicByRequest(HttpRequest * req, const KeyScope keyScope) { - StoreEntry *e = storeGetPublicByRequestMethod(req, req->method); + StoreEntry *e = storeGetPublicByRequestMethod(req, req->method, keyScope); if (e == NULL && req->method == Http::METHOD_HEAD) /* We can generate a HEAD reply from a cached GET object */ - e = storeGetPublicByRequestMethod(req, Http::METHOD_GET); + e = storeGetPublicByRequestMethod(req, Http::METHOD_GET, keyScope); return e; } @@ -653,10 +653,8 @@ } void -StoreEntry::setPublicKey() +StoreEntry::setPublicKey(const KeyScope scope) { - const cache_key *newkey; - if (key && !EBIT_TEST(flags, KEY_PRIVATE)) return; /* is already public */ @@ -680,80 +678,35 @@ assert(!EBIT_TEST(flags, RELEASE_REQUEST)); - if (mem_obj->request) { - HttpRequest *request = mem_obj->request; - - if (mem_obj->vary_headers.isEmpty()) { - /* First handle the case where the object no longer varies */ - request->vary_headers.clear(); - } else { - if (!request->vary_headers.isEmpty() && request->vary_headers.cmp(mem_obj->vary_headers) != 0) { - /* Oops.. the variance has changed. Kill the base object - * to record the new variance key - */ - request->vary_headers.clear(); /* free old "bad" variance key */ - if (StoreEntry *pe = storeGetPublic(mem_obj->storeId(), mem_obj->method)) - pe->release(); - } - - /* Make sure the request knows the variance status */ - if (request->vary_headers.isEmpty()) - request->vary_headers = httpMakeVaryMark(request, mem_obj->getReply()); - } - - // TODO: storeGetPublic() calls below may create unlocked entries. - // We should add/use storeHas() API or lock/unlock those entries. - if (!mem_obj->vary_headers.isEmpty() && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) { - /* Create "vary" base object */ - String vary; - StoreEntry *pe = storeCreateEntry(mem_obj->storeId(), mem_obj->logUri(), request->flags, request->method); - /* We are allowed to do this typecast */ - 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.size() > 0) { - /* Again, we own this structure layout */ - rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf()); - vary.clean(); - } - -#endif - pe->replaceHttpReply(rep, false); // no write until key is public - - pe->timestampsSet(); - - pe->makePublic(); - - pe->startWriting(); // after makePublic() - - pe->complete(); - - pe->unlock("StoreEntry::setPublicKey+Vary"); - } - - newkey = storeKeyPublicByRequest(mem_obj->request); - } else - newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method); - + adjustVary(); + forcePublicKey(calcPublicKey(scope)); +} + +void +StoreEntry::clearPublicKeyScope() +{ + if (!key || EBIT_TEST(flags, KEY_PRIVATE)) + return; // probably the old public key was deleted or made private + + // TODO: adjustVary() when collapsed revalidation supports that + + const cache_key *newKey = calcPublicKey(ksDefault); + if (!storeKeyHashCmp(key, newKey)) + return; // probably another collapsed revalidation beat us to this change + + forcePublicKey(newKey); +} + +/// Unconditionally sets public key for this store entry. +/// Releases the old entry with the same public key (if any). +void +StoreEntry::forcePublicKey(const cache_key *newkey) +{ if (StoreEntry *e2 = (StoreEntry *)hash_lookup(store_table, newkey)) { + assert(e2 != this); debugs(20, 3, "Making old " << *e2 << " private."); e2->setPrivateKey(); e2->release(); - - if (mem_obj->request) - newkey = storeKeyPublicByRequest(mem_obj->request); - else - newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method); } if (key) @@ -767,6 +720,84 @@ storeDirSwapLog(this, SWAP_LOG_ADD); } +/// Calculates correct public key for feeding forcePublicKey(). +/// Assumes adjustVary() has been called for this entry already. +const cache_key *StoreEntry::calcPublicKey(const KeyScope keyScope) { + assert(mem_obj); + return mem_obj->request ? storeKeyPublicByRequest(mem_obj->request, keyScope) : + storeKeyPublic(mem_obj->storeId(), mem_obj->method, keyScope); +} + +/// Updates mem_obj->request->vary_headers to reflect the current Vary. +/// The vary_headers field is used to calculate the Vary marker key. +/// Releases the old Vary marker with an outdated key (if any). +void StoreEntry::adjustVary() { + assert(mem_obj); + + if (!mem_obj->request) + return; + + HttpRequest *request = mem_obj->request; + + if (mem_obj->vary_headers.isEmpty()) { + /* First handle the case where the object no longer varies */ + request->vary_headers.clear(); + } else { + if (!request->vary_headers.isEmpty() && request->vary_headers.cmp(mem_obj->vary_headers) != 0) { + /* Oops.. the variance has changed. Kill the base object + * to record the new variance key + */ + request->vary_headers.clear(); /* free old "bad" variance key */ + if (StoreEntry *pe = storeGetPublic(mem_obj->storeId(), mem_obj->method)) + pe->release(); + } + + /* Make sure the request knows the variance status */ + if (request->vary_headers.isEmpty()) + request->vary_headers = httpMakeVaryMark(request, mem_obj->getReply()); + } + + // TODO: storeGetPublic() calls below may create unlocked entries. + // We should add/use storeHas() API or lock/unlock those entries. + if (!mem_obj->vary_headers.isEmpty() && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) { + /* Create "vary" base object */ + String vary; + StoreEntry *pe = storeCreateEntry(mem_obj->storeId(), mem_obj->logUri(), request->flags, request->method); + /* We are allowed to do this typecast */ + 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.size() > 0) { + /* Again, we own this structure layout */ + rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf()); + vary.clean(); + } + +#endif + pe->replaceHttpReply(rep, false); // no write until key is public + + pe->timestampsSet(); + + pe->makePublic(); + + pe->startWriting(); // after makePublic() + + pe->complete(); + + pe->unlock("StoreEntry::forcePublicKey+Vary"); + } +} + StoreEntry * storeCreatePureEntry(const char *url, const char *log_url, const RequestFlags &flags, const HttpRequestMethod& method) { @@ -1549,7 +1580,7 @@ return 1; } -void +bool StoreEntry::timestampsSet() { const HttpReply *reply = getReply(); @@ -1587,14 +1618,22 @@ served_date -= (squid_curtime - request_sent); } + time_t exp = 0; if (reply->expires > 0 && reply->date > -1) - expires = served_date + (reply->expires - reply->date); + exp = served_date + (reply->expires - reply->date); else - expires = reply->expires; + exp = reply->expires; + + if (lastmod == reply->last_modified && timestamp == served_date && expires == exp) + return false; + + expires = exp; lastmod = reply->last_modified; timestamp = served_date; + + return true; } void === modified file 'src/store_dir.cc' --- src/store_dir.cc 2016-01-01 00:14:27 +0000 +++ src/store_dir.cc 2016-09-23 15:28:42 +0000 @@ -77,7 +77,7 @@ debugs(47, DBG_IMPORTANT, "Using Least Load store dir selection"); } - if (UsingSmp() && IamWorkerProcess() && Config.onoff.collapsed_forwarding) { + if (UsingSmp() && IamWorkerProcess() && Config.onoff.collapsed_forwarding && smpAware()) { transients = new Transients; transients->init(); } @@ -916,7 +916,8 @@ StoreController::allowCollapsing(StoreEntry *e, const RequestFlags &reqFlags, const HttpRequestMethod &reqMethod) { - e->makePublic(); // this is needed for both local and SMP collapsing + const KeyScope keyScope = reqFlags.refresh ? ksRevalidation : ksDefault; + e->makePublic(keyScope); // this is needed for both local and SMP collapsing if (transients) transients->startWriting(e, reqFlags, reqMethod); debugs(20, 3, "may " << (transients && e->mem_obj->xitTable.index >= 0 ? @@ -1003,6 +1004,12 @@ return found; } +bool +StoreController::smpAware() const +{ + return memStore || (swapDir.getRaw() && swapDir->smpAware()); +} + StoreHashIndex::StoreHashIndex() { if (store_table) @@ -1267,6 +1274,19 @@ return new StoreSearchHashIndex (this); } +bool +StoreHashIndex::smpAware() const +{ + 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()) + return true; + } + return false; +} + CBDATA_CLASS_INIT(StoreSearchHashIndex); StoreSearchHashIndex::StoreSearchHashIndex(RefCount aSwapDir) : @@ -1345,4 +1365,3 @@ ++bucket; debugs(47,3, "got entries: " << entries.size()); } - === modified file 'src/store_key_md5.cc' --- src/store_key_md5.cc 2016-04-01 06:15:31 +0000 +++ src/store_key_md5.cc 2016-09-23 15:28:42 +0000 @@ -96,7 +96,7 @@ } const cache_key * -storeKeyPublic(const char *url, const HttpRequestMethod& method) +storeKeyPublic(const char *url, const HttpRequestMethod& method, const KeyScope keyScope) { static cache_key digest[SQUID_MD5_DIGEST_LENGTH]; unsigned char m = (unsigned char) method.id(); @@ -104,18 +104,20 @@ SquidMD5Init(&M); SquidMD5Update(&M, &m, sizeof(m)); SquidMD5Update(&M, (unsigned char *) url, strlen(url)); + if (keyScope) + SquidMD5Update(&M, &keyScope, sizeof(keyScope)); SquidMD5Final(digest, &M); return digest; } const cache_key * -storeKeyPublicByRequest(HttpRequest * request) +storeKeyPublicByRequest(HttpRequest * request, const KeyScope keyScope) { - return storeKeyPublicByRequestMethod(request, request->method); + return storeKeyPublicByRequestMethod(request, request->method, keyScope); } const cache_key * -storeKeyPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method) +storeKeyPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method, const KeyScope keyScope) { static cache_key digest[SQUID_MD5_DIGEST_LENGTH]; unsigned char m = (unsigned char) method.id(); @@ -125,6 +127,9 @@ SquidMD5Update(&M, &m, sizeof(m)); SquidMD5Update(&M, (unsigned char *) url, strlen(url)); + if (keyScope) + SquidMD5Update(&M, &keyScope, sizeof(keyScope)); + if (!request->vary_headers.isEmpty()) { SquidMD5Update(&M, request->vary_headers.rawContent(), request->vary_headers.length()); debugs(20, 3, "updating public key by vary headers: " << request->vary_headers << " for: " << url); === modified file 'src/store_key_md5.h' --- src/store_key_md5.h 2016-01-01 00:14:27 +0000 +++ src/store_key_md5.h 2016-09-23 15:28:42 +0000 @@ -17,14 +17,19 @@ class HttpRequestMethod; class HttpRequest; +typedef enum { + ksDefault, + ksRevalidation +} KeyScope; + cache_key *storeKeyDup(const cache_key *); cache_key *storeKeyCopy(cache_key *, const cache_key *); void storeKeyFree(const cache_key *); const cache_key *storeKeyScan(const char *); const char *storeKeyText(const cache_key *); -const cache_key *storeKeyPublic(const char *, const HttpRequestMethod&); -const cache_key *storeKeyPublicByRequest(HttpRequest *); -const cache_key *storeKeyPublicByRequestMethod(HttpRequest *, const HttpRequestMethod&); +const cache_key *storeKeyPublic(const char *, const HttpRequestMethod&, const KeyScope keyScope = ksDefault); +const cache_key *storeKeyPublicByRequest(HttpRequest *, const KeyScope keyScope = ksDefault); +const cache_key *storeKeyPublicByRequestMethod(HttpRequest *, const HttpRequestMethod&, const KeyScope keyScope = ksDefault); const cache_key *storeKeyPrivate(const char *, const HttpRequestMethod&, int); int storeKeyHashBuckets(int); int storeKeyNull(const cache_key *); === modified file 'src/tests/stub_store.cc' --- src/tests/stub_store.cc 2016-01-01 00:14:27 +0000 +++ src/tests/stub_store.cc 2016-09-23 15:28:42 +0000 @@ -42,9 +42,9 @@ void StoreEntry::trimMemory(const bool preserveSwappable) STUB void StoreEntry::abort() STUB void StoreEntry::unlink() STUB -void StoreEntry::makePublic() STUB +void StoreEntry::makePublic(const KeyScope keyScope) STUB void StoreEntry::makePrivate() STUB -void StoreEntry::setPublicKey() STUB +void StoreEntry::setPublicKey(const KeyScope keyScope) STUB void StoreEntry::setPrivateKey() STUB void StoreEntry::expireNow() STUB void StoreEntry::releaseRequest() STUB @@ -67,7 +67,7 @@ void StoreEntry::registerAbort(STABH * cb, void *) STUB void StoreEntry::reset() STUB void StoreEntry::setMemStatus(mem_status_t) STUB -void StoreEntry::timestampsSet() STUB +bool StoreEntry::timestampsSet() STUB_RETVAL(false) void StoreEntry::unregisterAbort() STUB void StoreEntry::destroyMemObject() STUB int StoreEntry::checkTooSmall() STUB_RETVAL(0) @@ -125,8 +125,8 @@ size_t storeEntryInUse() STUB_RETVAL(0) void storeEntryReplaceObject(StoreEntry *, HttpReply *) STUB StoreEntry *storeGetPublic(const char *uri, const HttpRequestMethod& method) STUB_RETVAL(NULL) -StoreEntry *storeGetPublicByRequest(HttpRequest * request) STUB_RETVAL(NULL) -StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method) STUB_RETVAL(NULL) +StoreEntry *storeGetPublicByRequest(HttpRequest * request, const KeyScope keyScope) STUB_RETVAL(NULL) +StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method, const KeyScope keyScope) STUB_RETVAL(NULL) StoreEntry *storeCreateEntry(const char *, const char *, const RequestFlags &, const HttpRequestMethod&) STUB_RETVAL(NULL) StoreEntry *storeCreatePureEntry(const char *storeId, const char *logUrl, const RequestFlags &, const HttpRequestMethod&) STUB_RETVAL(NULL) void storeInit(void) STUB === modified file 'src/tests/testRock.cc' --- src/tests/testRock.cc 2016-02-23 15:52:04 +0000 +++ src/tests/testRock.cc 2016-09-23 15:28:42 +0000 @@ -143,8 +143,6 @@ httpHeaderInitModule(); /* must go before any header processing (e.g. the one in errorInitialize) */ - httpReplyInitModule(); /* must go before accepting replies */ - mem_policy = createRemovalPolicy(Config.replPolicy); inited = true; === modified file 'src/tests/testStore.h' --- src/tests/testStore.h 2016-01-01 00:14:27 +0000 +++ src/tests/testStore.h 2016-09-23 15:28:42 +0000 @@ -76,6 +76,8 @@ virtual bool dereference(StoreEntry &, bool) { return true; } virtual StoreSearch *search(String const url, HttpRequest *); + + virtual bool smpAware() const { return false; } }; typedef RefCount TestStorePointer; === modified file 'src/tests/testUfs.cc' --- src/tests/testUfs.cc 2016-01-01 00:14:27 +0000 +++ src/tests/testUfs.cc 2016-09-23 15:28:42 +0000 @@ -75,8 +75,6 @@ httpHeaderInitModule(); /* must go before any header processing (e.g. the one in errorInitialize) */ - httpReplyInitModule(); /* must go before accepting replies */ - inited = true; }