From 3c856e95a672cd23da7eaebbfd07686dac4464ef Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 18 Apr 2014 10:57:01 -0600 Subject: [PATCH] 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). --- src/cache_cf.cc | 30 ------------------------------ src/globals.h | 2 +- src/store.cc | 26 ++++++++++++++++++++++++++ src/store_swapout.cc | 4 ++-- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/src/cache_cf.cc b/src/cache_cf.cc index 9d05a90585..66a1cac899 100644 --- 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(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(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 dd601bcbb9..328f4dabd0 100644 --- 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 916aad2b90..e4ecf40557 100644 --- 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(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(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 657d993ca7..457d342c09 100644 --- 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 -- 2.47.3