From: Amos Jeffries <squid3@treenet.co.nz>
Date: Thu, 3 Sep 2009 12:57:49 +0000 (+1200)
Subject: Author: Alex Rousskov <rousskov@measurement-factory.com>
X-Git-Tag: SQUID_3_0_STABLE19~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=556cea3c5e309959f898867d638aa68ac107b98d;p=thirdparty%2Fsquid.git

Author: Alex Rousskov <rousskov@measurement-factory.com>
Fixed entry size calculation for the max-size cache_dir selection algorithms.

There were two sides of this bug:

In src/store_swapout.cc, we must create metadata earlier because
storeCreate() needs swap_hdr_sz. With swap_hdr_sz unknown at the time of
storeCreate(), the SwapDir selection algorithms may select SwapDirs that
should not really take the entry as the real storage size (with the
metadata swap_hdr_sz) would exceed the store slot size.

In src/store_dir.cc, we must add the metadata size before looking for
cache_dirs that accept objsize. Only the "new"
storeDirSelectSwapDirRoundRobin selection scheme was affected.
---

diff --git a/src/store_dir.cc b/src/store_dir.cc
index e3b7ae3e80..087c7e2416 100644
--- a/src/store_dir.cc
+++ b/src/store_dir.cc
@@ -189,6 +189,10 @@ storeDirSelectSwapDirRoundRobin(const StoreEntry * e)
     int load;
     RefCount<SwapDir> sd;
 
+    ssize_t objsize = e->objectLen();
+    if (objsize != -1)
+        objsize += e->mem_obj->swap_hdr_sz;
+
     for (i = 0; i <= Config.cacheSwap.n_configured; i++) {
         if (++dirn >= Config.cacheSwap.n_configured)
             dirn = 0;
@@ -201,7 +205,7 @@ storeDirSelectSwapDirRoundRobin(const StoreEntry * e)
         if (sd->cur_size > sd->max_size)
             continue;
 
-        if (!sd->objectSizeIsAcceptable(e->objectLen()))
+        if (!sd->objectSizeIsAcceptable(objsize))
             continue;
 
         /* check for error or overload condition */
diff --git a/src/store_swapout.cc b/src/store_swapout.cc
index 75ccf07693..3ba12d7de1 100644
--- a/src/store_swapout.cc
+++ b/src/store_swapout.cc
@@ -63,6 +63,15 @@ storeSwapOutStart(StoreEntry * e)
     /* If we start swapping out objects with OutOfBand Metadata,
      * then this code needs changing
      */
+
+    /* TODO: make some sort of data,size refcounted immutable buffer
+     * and stop fooling ourselves with "const char*" buffers.
+     */
+
+    // Create metadata now, possibly in vain: storeCreate needs swap_hdr_sz.
+    const char *buf = e->getSerialisedMetaData ();
+    assert(buf);
+
     /* Create the swap file */
     generic_cbdata *c = new generic_cbdata(e);
     sio = storeCreate(e, storeSwapOutFileNotify, storeSwapOutFileClosed, c);
@@ -70,6 +79,7 @@ storeSwapOutStart(StoreEntry * e)
     if (sio == NULL) {
         e->swap_status = SWAPOUT_NONE;
         delete c;
+        xfree((char*)buf);
         storeLog(STORE_LOG_SWAPOUTFAIL, e);
         return;
     }
@@ -87,16 +97,6 @@ storeSwapOutStart(StoreEntry * e)
     e->swap_dirn = mem->swapout.sio->swap_dirn;
 
     /* write out the swap metadata */
-    /* TODO: make some sort of data,size refcounted immutable buffer
-     * for use by this sort of function.
-     */
-    char const *buf = e->getSerialisedMetaData ();
-
-    /* If we start swapping out with out of band metadata, this assert
-     * will catch it - this code needs to be adjusted if that happens
-     */
-    assert (buf);
-
     storeIOWrite(mem->swapout.sio, buf, mem->swap_hdr_sz, 0, xfree);
 }