]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Collapse internal revalidation requests (SMP-unaware caches).
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Sat, 23 Jul 2016 13:36:56 +0000 (01:36 +1200)
committerAmos Jeffries <squid3@treenet.co.nz>
Sat, 23 Jul 2016 13:36:56 +0000 (01:36 +1200)
... 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.

24 files changed:
src/HttpHeader.cc
src/HttpHeader.h
src/HttpReply.cc
src/HttpReply.h
src/MemStore.h
src/Store.h
src/Transients.cc
src/Transients.h
src/cf.data.pre
src/client_side_reply.cc
src/client_side_reply.h
src/fs/rock/RockSwapDir.h
src/fs/ufs/UFSSwapDir.h
src/store.cc
src/store/Controller.cc
src/store/Controller.h
src/store/Disk.h
src/store/Disks.cc
src/store/Disks.h
src/store/Storage.h
src/store_key_md5.cc
src/store_key_md5.h
src/tests/stub_HttpReply.cc
src/tests/stub_store.cc

index cc26f255e0ffae9dd78710720d3bbb96ffb81723..59e8b30a97914afe1e23985ce293e9ad0ec90d7e 100644 (file)
@@ -238,6 +238,22 @@ HttpHeader::append(const HttpHeader * src)
     }
 }
 
+/// check whether the fresh header has any new/changed updatable fields
+bool
+HttpHeader::needUpdate(HttpHeader const *fresh) const
+{
+    for (const auto e: fresh->entries) {
+        if (skipUpdateHeader(e->id))
+            continue;
+        String value;
+        const char *name = e->name.termedBuf();
+        if (!getByNameIfPresent(name, strlen(name), value) ||
+                (value != fresh->getByName(name)))
+            return true;
+    }
+    return false;
+}
+
 void
 HttpHeader::updateWarnings()
 {
@@ -258,16 +274,22 @@ HttpHeader::skipUpdateHeader(const Http::HdrType id) const
     return id == Http::HdrType::WARNING;
 }
 
-void
+bool
 HttpHeader::update(HttpHeader const *fresh)
 {
-    const HttpHeaderEntry *e;
-    HttpHeaderPos pos = HttpHeaderInitPos;
     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 Http::HdrType::OTHER) here */
 
@@ -291,6 +313,7 @@ HttpHeader::update(HttpHeader const *fresh)
 
         addEntry(e->clone());
     }
+    return true;
 }
 
 int
index 4c9655111d70ff5b06d48cc3eb43802eaba141b1..4ecb9b37486ccd5508f956c0ed8d250db3e2268f 100644 (file)
@@ -81,7 +81,7 @@ public:
     /* Interface functions */
     void clean();
     void append(const HttpHeader * src);
-    void update(HttpHeader const *fresh);
+    bool update(HttpHeader const *fresh);
     void compact();
     int parse(const char *header_start, size_t len);
     void packInto(Packable * p, bool mask_sensitive_info=false) const;
@@ -145,6 +145,7 @@ public:
 protected:
     /** \deprecated Public access replaced by removeHopByHopEntries() */
     void removeConnectionHeaderEntries();
+    bool needUpdate(const HttpHeader *fresh) const;
     bool skipUpdateHeader(const Http::HdrType id) const;
     void updateWarnings();
 
index e40a212cb293cea9e2a4de4c897fed6b142b8f60..9a6b0f152955580f6da152cee95113e1a70baea0 100644 (file)
@@ -235,19 +235,23 @@ HttpReply::validatorsMatch(HttpReply const * otherRep) const
     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);
 
     header.compact();
     /* init cache */
     hdrCacheInit();
+
+    return true;
 }
 
 /* internal routines */
index 55e9de2239b462336b300f6d787fadbe64f28c91..1fe8bebc5d867ba41282a112149a6ec8e34d1bb2 100644 (file)
@@ -72,7 +72,7 @@ public:
 
     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,
index 6214b50e21be0099f96809ee4ecc762ed0939edc..a338193bdc205182b947959b8ad41e43eed203d2 100644 (file)
@@ -63,6 +63,7 @@ public:
     virtual bool updateCollapsed(StoreEntry &e) override;
     virtual void markForUnlink(StoreEntry &) override;
     virtual void unlink(StoreEntry &e) override;
+    virtual bool smpAware() const override { return true; }
 
     static int64_t EntryLimit();
 
index 4f1aff44bf2f1e6933c4cac9e62c75ad67141b80..f0b107236b519e67a0f3ca2c477fe307ba913d1e 100644 (file)
@@ -82,9 +82,13 @@ public:
     void swapOutDecision(const MemObject::SwapOut::Decision &decision);
 
     void abort();
-    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();
@@ -119,7 +123,7 @@ public:
     void registerAbort(STABH * cb, void *);
     void reset();
     void setMemStatus(mem_status_t);
-    void timestampsSet();
+    bool timestampsSet();
     void unregisterAbort();
     void destroyMemObject();
     int checkTooSmall();
@@ -217,6 +221,9 @@ protected:
 
 private:
     bool checkTooBig() const;
+    void forcePublicKey(const cache_key *newkey);
+    void adjustVary();
+    const cache_key *calcPublicKey(const KeyScope keyScope);
 
     static MemAllocator *pool;
 
@@ -286,10 +293,10 @@ void storeEntryReplaceObject(StoreEntry *, HttpReply *);
 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.
index acb5029e5ff4e2e100bdaa09d972ec740a0bcc14..c971d12f2232169f7b66f3f09e5a8802e44d95fb 100644 (file)
@@ -184,7 +184,8 @@ Transients::copyFromShm(const sfileno index)
     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
index 3054b38c854ae3507ca4101409479ed679a61517..1d65366ba09b2045275810f06526b88a6f299f87 100644 (file)
@@ -72,6 +72,7 @@ public:
     virtual void markForUnlink(StoreEntry &e) override;
     virtual void unlink(StoreEntry &e) override;
     virtual void maintain() override;
+    virtual bool smpAware() const override { return true; }
 
     static int64_t EntryLimit();
 
index 3a43f68983ce0648f616efa4fae74c297bee6c0c..a7604e387b04953a99ad05e5f2b5a718a8043bba 100644 (file)
@@ -6200,13 +6200,29 @@ DOC_START
        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
 
 NAME: collapsed_forwarding_shared_entries_limit
index eac883a5a6a590b74300105d97e1da9e23e4be73..e4ee2cc6310be17f9f3ef6926e999ffee81e5bed 100644 (file)
@@ -89,7 +89,8 @@ clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) :
     reply(NULL),
     old_entry(NULL),
     old_sc(NULL),
-    deleting(false)
+    deleting(false),
+    collapsedRevalidation(crNone)
 {
     *tempbuf = 0;
 }
@@ -264,7 +265,6 @@ void
 clientReplyContext::processExpired()
 {
     const char *url = storeId();
-    StoreEntry *entry = NULL;
     debugs(88, 3, "clientReplyContext::processExpired: '" << http->uri << "'");
     assert(http->storeEntry()->lastmod >= 0);
     /*
@@ -287,9 +287,36 @@ clientReplyContext::processExpired()
 #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 for 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 */
@@ -309,13 +336,14 @@ clientReplyContext::processExpired()
     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. */
 
     if (EBIT_TEST(entry->flags, ENTRY_ABORTED))
@@ -427,6 +455,10 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
         // 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();
     }
 
index 3ff26e4b5c77e3a4c74e8e41ab94a7013596eea0..af441c0c90b53e104c17f1e0a4bb4f815df40bbc 100644 (file)
@@ -133,6 +133,14 @@ private:
     StoreEntry *old_entry;
     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;
 };
 
 #endif /* SQUID_CLIENTSIDEREPLY_H */
index 7340ec7e4becae87cb261b57dc1770ffcd7342e3..57c8ab4608d402f5cb0cd3c816052a820c2080f4 100644 (file)
@@ -47,6 +47,7 @@ public:
     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;
index d26c921c0911def2a5f2173c260c6ccd3f60a947..72b3788b8639eecfdc9efaf6b944199d769f2f61 100644 (file)
@@ -71,6 +71,7 @@ public:
     virtual uint64_t currentSize() const override { return cur_size; }
     virtual uint64_t currentCount() const override { return n_disk_objects; }
     virtual ConfigOption *getOptionTree() const override;
+    virtual bool smpAware() const override { return false; }
 
     void unlinkFile(sfileno f);
     // move down when unlink is a virtual method
index b08f4134ad02392ed945a2321ecf971bbd733d22..494eb3cb5f0f26f32a421f997e9648db38b49db1 100644 (file)
@@ -139,12 +139,11 @@ StoreEntry::operator delete (void *address)
 }
 
 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
@@ -559,19 +558,19 @@ storeGetPublic(const char *uri, const HttpRequestMethod& method)
 }
 
 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;
 }
@@ -622,10 +621,8 @@ StoreEntry::setPrivateKey()
 }
 
 void
-StoreEntry::setPublicKey()
+StoreEntry::setPublicKey(const KeyScope scope)
 {
-    const cache_key *newkey;
-
     if (key && !EBIT_TEST(flags, KEY_PRIVATE))
         return;                 /* is already public */
 
@@ -649,80 +646,35 @@ StoreEntry::setPublicKey()
 
     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(Http::HdrType::VARY);
-
-            if (vary.size()) {
-                /* Again, we own this structure layout */
-                rep->header.putStr(Http::HdrType::VARY, vary.termedBuf());
-                vary.clean();
-            }
-
-#if X_ACCELERATOR_VARY
-            vary = mem_obj->getReply()->header.getList(Http::HdrType::HDR_X_ACCELERATOR_VARY);
-
-            if (vary.size() > 0) {
-                /* Again, we own this structure layout */
-                rep->header.putStr(Http::HdrType::HDR_X_ACCELERATOR_VARY, vary.termedBuf());
-                vary.clean();
-            }
-
-#endif
-            pe->replaceHttpReply(rep, false); // no write until key is public
-
-            pe->timestampsSet();
-
-            pe->makePublic();
+    adjustVary();
+    forcePublicKey(calcPublicKey(scope));
+}
 
-            pe->startWriting(); // after makePublic()
+void
+StoreEntry::clearPublicKeyScope()
+{
+    if (!key || EBIT_TEST(flags, KEY_PRIVATE))
+        return; // probably the old public key was deleted or made private
 
-            pe->complete();
+    // TODO: adjustVary() when collapsed revalidation supports that
 
-            pe->unlock("StoreEntry::setPublicKey+Vary");
-        }
+    const cache_key *newKey = calcPublicKey(ksDefault);
+    if (!storeKeyHashCmp(key, newKey))
+        return; // probably another collapsed revalidation beat us to this change
 
-        newkey = storeKeyPublicByRequest(mem_obj->request);
-    } else
-        newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method);
+    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)
@@ -736,6 +688,88 @@ StoreEntry::setPublicKey()
         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(Http::HdrType::VARY);
+
+        if (vary.size()) {
+            /* Again, we own this structure layout */
+            rep->header.putStr(Http::HdrType::VARY, vary.termedBuf());
+            vary.clean();
+        }
+
+#if X_ACCELERATOR_VARY
+        vary = mem_obj->getReply()->header.getList(Http::HdrType::HDR_X_ACCELERATOR_VARY);
+
+        if (vary.size() > 0) {
+            /* Again, we own this structure layout */
+            rep->header.putStr(Http::HdrType::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)
 {
@@ -1486,7 +1520,7 @@ StoreEntry::validToSend() const
     return 1;
 }
 
-void
+bool
 StoreEntry::timestampsSet()
 {
     const HttpReply *reply = getReply();
@@ -1524,14 +1558,22 @@ StoreEntry::timestampsSet()
             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
index 352370c6e65680e0bb339bf8135b181649ea95fd..3a2bd99a4ed2510aa4752ece6f988a6932cc159a 100644 (file)
@@ -64,7 +64,8 @@ Store::Controller::init()
 
     swapDir->init();
 
-    if (UsingSmp() && IamWorkerProcess() && Config.onoff.collapsed_forwarding) {
+    if (UsingSmp() && IamWorkerProcess() && Config.onoff.collapsed_forwarding &&
+            smpAware()) {
         transients = new Transients;
         transients->init();
     }
@@ -498,8 +499,10 @@ Store::Controller::updateOnNotModified(StoreEntry *old, const StoreEntry &newer)
     Must(old);
     HttpReply *oldReply = const_cast<HttpReply*>(old->getReply());
     Must(oldReply);
-    oldReply->updateOnNotModified(newer.getReply());
-    old->timestampsSet();
+
+    const bool modified = oldReply->updateOnNotModified(newer.getReply());
+    if (!old->timestampsSet() && !modified)
+        return;
 
     /* update stored image of the old entry */
 
@@ -514,7 +517,8 @@ void
 Store::Controller::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 ?
@@ -601,6 +605,12 @@ Store::Controller::anchorCollapsed(StoreEntry &collapsed, bool &inSync)
     return found;
 }
 
+bool
+Store::Controller::smpAware() const
+{
+    return memStore || (swapDir && swapDir->smpAware());
+}
+
 namespace Store {
 static RefCount<Controller> TheRoot;
 }
index 71e4eebf889919a12f500d8c18cde2555768315c..9f15a7d4c30972a0bbb3a4f909069354a6e3feb8 100644 (file)
@@ -41,6 +41,7 @@ public:
     virtual void markForUnlink(StoreEntry &) override;
     virtual void unlink(StoreEntry &) override;
     virtual int callback() override;
+    virtual bool smpAware() const override;
 
     /// Additional unknown-size entry bytes required by Store in order to
     /// reduce the risk of selecting the wrong disk cache for the growing entry.
index 4b47b49048a98d609b4063ebe7a99f705970385e..d937dbbc407959c0dd290876a3e4347ba9e30e65 100644 (file)
@@ -71,6 +71,8 @@ public:
     /// called when entry swap out is complete
     virtual void swappedOut(const StoreEntry &e) = 0;
 
+    virtual bool smpAware() const { return false; }
+
 protected:
     void parseOptions(int reconfiguring);
     void dumpOptions(StoreEntry * e) const;
index 4748c392a1663d740e21e8ab0e871e319c4c177e..790c11e00290232faa50e3f1ac108e964608aad8 100644 (file)
@@ -530,6 +530,19 @@ Store::Disks::updateCollapsed(StoreEntry &collapsed)
            dir(collapsed.swap_dirn).updateCollapsed(collapsed);
 }
 
+bool
+Store::Disks::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;
+}
+
 /* Store::Disks globals that should be converted to use RegisteredRunner */
 
 void
index 0510089ff57c6e65a5ee7ab873fdddaf8d456431..8314edd4c5cf7804f37729888d147170f3e518be 100644 (file)
@@ -48,6 +48,7 @@ public:
     /// Additional unknown-size entry bytes required by disks in order to
     /// reduce the risk of selecting the wrong disk cache for the growing entry.
     int64_t accumulateMore(const StoreEntry&) const;
+    virtual bool smpAware() const override;
 
 private:
     /* migration logic */
index f4264c70d29da8b8831acf259541ffc2eacae6eb..00aa4abd0bce73e3bd6a40f504cdc4c48995a2af 100644 (file)
@@ -74,6 +74,11 @@ public:
 
     /// prepare for shutdown
     virtual void sync() {}
+
+    /// whether this storage is capable of serving multiple workers;
+    /// a true result does not imply [lack of] non-SMP support because
+    /// [only] some SMP-aware storages also support non-SMP configss
+    virtual bool smpAware() const = 0;
 };
 
 } // namespace Store
index 23c22315c5f7e562ce04b67ee9d75bad8bbfec8f..818429f58c1ffc9e094ab9c087e4102c7f3a80c7 100644 (file)
@@ -95,7 +95,7 @@ storeKeyPrivate()
 }
 
 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();
@@ -103,18 +103,20 @@ storeKeyPublic(const char *url, const HttpRequestMethod& method)
     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();
@@ -123,6 +125,8 @@ storeKeyPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& me
     SquidMD5Init(&M);
     SquidMD5Update(&M, &m, sizeof(m));
     SquidMD5Update(&M, (unsigned char *) url.rawContent(), url.length());
+    if (keyScope)
+        SquidMD5Update(&M, &keyScope, sizeof(keyScope));
 
     if (!request->vary_headers.isEmpty()) {
         SquidMD5Update(&M, request->vary_headers.rawContent(), request->vary_headers.length());
index 27aba0bd42510ab8610bc432c614cd15fb0bcc48..fb2fa6709889b0782c715f00fdb9cff2d3273223 100644 (file)
 class HttpRequestMethod;
 class HttpRequest;
 
+typedef enum {
+    ksDefault = 0,
+    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();
 int storeKeyHashBuckets(int);
 int storeKeyNull(const cache_key *);
index 196ede285efe571f825bc6005b55fe4b171e4305..500623babd01c1ee10685dbdb76b0ef28447660d 100644 (file)
@@ -28,6 +28,6 @@ HttpReply::HttpReply() : HttpMsg(hoReply), date (0), last_modified (0),
     void HttpReply::hdrCacheInit() STUB
     HttpReply * HttpReply::clone() const STUB_RETVAL(NULL)
     bool HttpReply::inheritProperties(const HttpMsg *aMsg) STUB_RETVAL(false)
-    void HttpReply::updateOnNotModified(HttpReply const*) STUB
+    bool HttpReply::updateOnNotModified(HttpReply const*) STUB_RETVAL(false)
     int64_t HttpReply::bodySize(const HttpRequestMethod&) const STUB_RETVAL(0)
 
index ee4c891b214951a9cef5137fa7abb7043167e235..61219ff76c781d6987943a9c4e100bd20206c0db 100644 (file)
@@ -37,9 +37,9 @@ void StoreEntry::replaceHttpReply(HttpReply *, bool andStartWriting) STUB
 bool StoreEntry::mayStartSwapOut() STUB_RETVAL(false)
 void StoreEntry::trimMemory(const bool preserveSwappable) STUB
 void StoreEntry::abort() STUB
-void StoreEntry::makePublic() STUB
+void StoreEntry::makePublic(const KeyScope scope) STUB
 void StoreEntry::makePrivate() STUB
-void StoreEntry::setPublicKey() STUB
+void StoreEntry::setPublicKey(const KeyScope scope) STUB
 void StoreEntry::setPrivateKey() STUB
 void StoreEntry::expireNow() STUB
 void StoreEntry::releaseRequest() STUB
@@ -62,7 +62,7 @@ void StoreEntry::hashInsert(const cache_key *) STUB
 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)
@@ -121,8 +121,8 @@ std::ostream &operator <<(std::ostream &os, const StoreEntry &)
 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 scope) STUB_RETVAL(NULL)
+StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method, const KeyScope scope) 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 storeConfigure(void) STUB