Restored Squid ability to cache (in memory) when no disk caches are configured

which was lost during r12662 "Bug 3686: cache_dir max-size default fails"

The bug was hidden until the memory cache code started calling
StoreEntry::checkCachable() in branch r13315, exposing the entry size check to
a broken limit.

r12662 converted store_maxobjsize from a sometimes present disk-only limit to
an always set Squid-global limit. However, store_maxobjsize value was only
calculated when parsing cache_dirs. A config without cache_dirs would leave
store_maxobjsize at -1, triggering "StoreEntry::checkCachable: NO: too big"
prohibition for all responses.

This change moves store_maxobjsize calculation from parser to storeConfigure()
where some other Store globals are computed after squid.conf parsing.

Also honored memory cache size limit (just in case cache_mem is smaller than
maximum_object_size_in_memory) and removed leftover checks for
store_maxobjsize being set (it should always be set, at least to zero).

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 9d05a90585acd068298c0d274c2c256f716434d0..66a1cac899ca5f218ee920c96e02959ac6efb84b 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -177,7 +177,6 @@ static void dump_access_log(StoreEntry * entry, const char *name, CustomLog * de
 static void free_access_log(CustomLog ** definitions);
 static bool setLogformat(CustomLog *cl, const char *name, const bool dieWhenMissing);
 
-static void update_maxobjsize(void);
 static void configDoConfigure(void);
 static void parse_refreshpattern(RefreshPattern **);
 static uint64_t parseTimeUnits(const char *unit,  bool allowMsec);
@@ -272,29 +271,6 @@ self_destruct(void)
     LegacyParser.destruct();
 }
 
-static void
-update_maxobjsize(void)
-{
-    int64_t ms = -1;
-
-    // determine the maximum size object that can be stored to disk
-    for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
-        assert (Config.cacheSwap.swapDirs[i].getRaw());
-
-        const int64_t storeMax = dynamic_cast<SwapDir *>(Config.cacheSwap.swapDirs[i].getRaw())->maxObjectSize();
-        if (ms < storeMax)
-            ms = storeMax;
-    }
-
-    // Ensure that we do not discard objects which could be stored only in memory.
-    // It is governed by maximum_object_size_in_memory (for now)
-    // TODO: update this to check each in-memory location (SMP and local memory limits differ)
-    if (ms < static_cast<int64_t>(Config.Store.maxInMemObjSize))
-        ms = Config.Store.maxInMemObjSize;
-
-    store_maxobjsize = ms;
-}
-
 static void
 SetConfigFilename(char const *file_name, bool is_pipe)
 {
@@ -1943,9 +1919,6 @@ parse_cachedir(SquidConfig::_cacheSwap * swap)
             }
 
             sd->reconfigure();
-
-            update_maxobjsize();
-
             return;
         }
     }
@@ -1969,9 +1942,6 @@ parse_cachedir(SquidConfig::_cacheSwap * swap)
     sd->parse(swap->n_configured, path_str);
 
     ++swap->n_configured;
-
-    /* Update the max object size */
-    update_maxobjsize();
 }
 
 static const char *

diff --git a/src/globals.h b/src/globals.h
index dd601bcbb9c1f9bce49dacc7e5afeff03d8e1a42..328f4dabd0babe09713a832e4029ac62e1954769 100644 (file)
--- a/src/globals.h
+++ b/src/globals.h
@@ -109,7 +109,7 @@ extern const char *SwapDirType[];
 extern int store_swap_low;     /* 0 */
 extern int store_swap_high;    /* 0 */
 extern size_t store_pages_max; /* 0 */
-extern int64_t store_maxobjsize;       /* -1 */
+extern int64_t store_maxobjsize;       /* 0 */
 extern hash_table *proxy_auth_username_cache;  /* NULL */
 extern int incoming_sockets_accepted;
 #if _SQUID_WINDOWS_

diff --git a/src/store.cc b/src/store.cc
index 916aad2b9030b1fba093db3e2b2fab1ec85951a6..e4ecf40557fa0b640640c89b8b5d17b464cea8df 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -1398,6 +1398,30 @@ storeInit(void)
     storeRegisterWithCacheManager();
 }
 
+/// computes maximum size of a cachable object
+/// larger objects are rejected by all (disk and memory) cache stores
+static int64_t
+storeCalcMaxObjSize()
+{
+    int64_t ms = 0; // nothing can be cached without at least one store consent
+
+    // global maximum is at least the disk store maximum
+    for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
+        assert (Config.cacheSwap.swapDirs[i].getRaw());
+        const int64_t storeMax = dynamic_cast<SwapDir *>(Config.cacheSwap.swapDirs[i].getRaw())->maxObjectSize();
+        if (ms < storeMax)
+            ms = storeMax;
+    }
+
+    // global maximum is at least the memory store maximum
+    // TODO: move this into a memory cache class when we have one
+    const int64_t memMax = static_cast<int64_t>(min(Config.Store.maxInMemObjSize, Config.memMaxSize));
+    if (ms < memMax)
+        ms = memMax;
+
+    return ms;
+}
+
 void
 storeConfigure(void)
 {
@@ -1406,6 +1430,8 @@ storeConfigure(void)
     store_swap_low = (long) (((float) Store::Root().maxSize() *
                               (float) Config.Swap.lowWaterMark) / (float) 100);
     store_pages_max = Config.memMaxSize / sizeof(mem_node);
+
+    store_maxobjsize = storeCalcMaxObjSize();
 }
 
 bool

diff --git a/src/store_swapout.cc b/src/store_swapout.cc
index 657d993ca76e9c27d48e529c04605c7ad6ab8b4e..457d342c09a6cbe13580125557f138d3ee6d72d2 100644 (file)
--- a/src/store_swapout.cc
+++ b/src/store_swapout.cc
@@ -423,8 +423,8 @@ StoreEntry::mayStartSwapOut()
         return false;
     }
 
-    // check cache_dir max-size limit if all cache_dirs have it
-    if (store_maxobjsize >= 0) {
+    // handle store_maxobjsize limit
+    {
         // TODO: add estimated store metadata size to be conservative
 
         // use guaranteed maximum if it is known