From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Thu, 28 Apr 2011 22:45:55 +0000 (-0600)
Subject: Extraced the write-to-store step from StoreEntry::replaceHttpReply().
X-Git-Tag: take07~25
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=3756e5c03bed4a1563242487e208125bd86974a0;p=thirdparty%2Fsquid.git

Extraced the write-to-store step from StoreEntry::replaceHttpReply().

This allows the caller to set the reply for the entry and then update the
entry and the reply before writing them to store. For example, the server-side
haveParsedReplyHeaders() code needs to set the entry timestamps and make the
entry key public before the entry starts swapping out, but the same code also
needs access to entry->getReply() and such for timestampsSet() and similar
code to work correctly.

TODO: Calls to StoreEntry::replaceHttpReply() do not have to be modified
because replaceHttpReply() does write by default. However, it is likely that
callers other than ServerStateData::setFinalReply() should take advantage of
the new split interface because they call timestampsSet() and such after
replaceHttpReply().
---

diff --git a/src/MemObject.cc b/src/MemObject.cc
index 9f9055fd9e..750e7e7bbf 100644
--- a/src/MemObject.cc
+++ b/src/MemObject.cc
@@ -244,6 +244,15 @@ MemObject::endOffset () const
     return data_hdr.endOffset();
 }
 
+void
+MemObject::markEndOfReplyHeaders()
+{
+    const int hdr_sz = endOffset();
+    assert(hdr_sz >= 0);
+    assert(_reply);
+    _reply->hdr_sz = hdr_sz;
+}
+
 int64_t
 MemObject::size() const
 {
diff --git a/src/MemObject.h b/src/MemObject.h
index fc2f0b2708..f02ace5521 100644
--- a/src/MemObject.h
+++ b/src/MemObject.h
@@ -67,6 +67,7 @@ public:
     void replaceHttpReply(HttpReply *newrep);
     void stat (MemBuf * mb) const;
     int64_t endOffset () const;
+    void markEndOfReplyHeaders(); ///< sets _reply->hdr_sz to endOffset()
     /// negative if unknown; otherwise, expected object_sz, expected endOffset
     /// maximum, and stored reply headers+body size (all three are the same)
     int64_t expectedReplySize() const;
diff --git a/src/Server.cc b/src/Server.cc
index 5baf389a1e..88d2e81dfb 100644
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -162,8 +162,10 @@ ServerStateData::setFinalReply(HttpReply *rep)
     assert(rep);
     theFinalReply = HTTPMSGLOCK(rep);
 
-    haveParsedReplyHeaders();
-    entry->replaceHttpReply(theFinalReply);
+    // give entry the reply because haveParsedReplyHeaders() expects it there
+    entry->replaceHttpReply(theFinalReply, false); // but do not write yet
+    haveParsedReplyHeaders(); // update the entry/reply (e.g., set timestamps)
+    entry->startWriting(); // write the updated entry to store
 
     return theFinalReply;
 }
diff --git a/src/Store.h b/src/Store.h
index 22d0615fac..ccb50f8ab4 100644
--- a/src/Store.h
+++ b/src/Store.h
@@ -95,7 +95,8 @@ public:
     virtual void complete();
     virtual store_client_t storeClientType() const;
     virtual char const *getSerialisedMetaData();
-    virtual void replaceHttpReply(HttpReply *);
+    void replaceHttpReply(HttpReply *, bool andStartWriting = true);
+    void startWriting(); ///< pack and write reply headers and, maybe, body
     virtual bool swapoutPossible();
     virtual void trimMemory();
     void abort();
diff --git a/src/refresh.cc b/src/refresh.cc
index 4617102a8e..d16915a401 100644
--- a/src/refresh.cc
+++ b/src/refresh.cc
@@ -392,7 +392,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta)
      * NOTE: max-stale config blocks the overrides.
      */
     int max_stale = (R->max_stale >= 0 ? R->max_stale : Config.maxStale);
-    if ( max_stale >= 0 && staleness < max_stale) {
+    if ( max_stale >= 0 && staleness > max_stale) {
         debugs(22, 3, "refreshCheck: YES: max-stale limit");
         if (request)
             request->flags.fail_on_validation_err = 1;
diff --git a/src/store.cc b/src/store.cc
index e6211bf096..2113b126ab 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -1816,10 +1816,9 @@ storeSwapFileNumberSet(StoreEntry * e, sfileno filn)
  * a new reply. This eats the reply.
  */
 void
-StoreEntry::replaceHttpReply(HttpReply *rep)
+StoreEntry::replaceHttpReply(HttpReply *rep, bool andStartWriting)
 {
     debugs(20, 3, "StoreEntry::replaceHttpReply: " << url());
-    Packer p;
 
     if (!mem_obj) {
         debugs(20, 0, "Attempt to replace object with no in-memory representation");
@@ -1828,18 +1827,31 @@ StoreEntry::replaceHttpReply(HttpReply *rep)
 
     mem_obj->replaceHttpReply(rep);
 
+    if (andStartWriting)
+        startWriting();
+}
+
+
+void
+StoreEntry::startWriting()
+{
+    Packer p;
+
     /* TODO: when we store headers serparately remove the header portion */
     /* TODO: mark the length of the headers ? */
     /* We ONLY want the headers */
     packerToStoreInit(&p, this);
 
     assert (isEmpty());
+    assert(mem_obj);
+    
+    const HttpReply *rep = getReply();
+    assert(rep);
 
-    getReply()->packHeadersInto(&p);
-
-    rep->hdr_sz = mem_obj->endOffset();
+    rep->packHeadersInto(&p);
+    mem_obj->markEndOfReplyHeaders();
 
-    httpBodyPackInto(&getReply()->body, &p);
+    httpBodyPackInto(&rep->body, &p);
 
     packerClean(&p);
 }