http://bugs.squid-cache.org/show_bug.cgi?id=3686
The maximum cached object size gets capped to the highest specified
max-size of any cache_dir, even if there is cache_dir available with no
max-size specified.
The solution:
Re-document the global as a default limit with cache_dir specific values
overriding it for that store. And re-implement it strictly as a default
only applied when a cache_dir has no explicit max-size setting.
After this patch:
- maximum_object_size is converted into the default value for cache_dir
max-size= options when none is supplied.
- every cache_dir will always produce a max size for that storage area.
This is accessible as StoreDir::maxObjectSize().
- no cache_dir may be configured with a larger max-size= than will fit
in that storage area.
- cache_dir may be configured with max-size=0 which will result in no
new additions being made, *as well* as any form of revalidation purging
existing entries as they are detected to be "too big" for the area.
NP: use no-store option to make a cache_dir read-only without this
purging behaviour.
- Squid will not cache any object larger than the *largest* storage
maximum (including memory cache maximum_object_size_in_memory). This is
currently accessible as store_maxobjsize.
- if no cache_dir are specified, the largest cached object will be
limited by maximum_object_size_in_memory.
Amos
=== modified file 'src/SwapDir.cc'
--- src/SwapDir.cc 2013-01-31 15:05:34 +0000
+++ src/SwapDir.cc 2013-02-05 05:39:40 +0000
@@ -42,8 +42,8 @@
#include "tools.h"
SwapDir::SwapDir(char const *aType): theType(aType),
- max_size(0),
- path(NULL), index(-1), disker(-1), min_objsize(0), max_objsize (-1),
+ max_size(0), min_objsize(0), max_objsize (-1),
+ path(NULL), index(-1), disker(-1),
repl(NULL), removals(0), scanned(0),
cleanLog(NULL)
{
@@ -114,6 +114,38 @@
return ((maxSize() * Config.Swap.lowWaterMark) / 100);
}
+int64_t
+SwapDir::maxObjectSize() const
+{
+ // per-store max-size=N value is authoritative
+ if (max_objsize > -1)
+ return max_objsize;
+
+ // store with no individual max limit is limited by configured
maximum_object_size
+ // or the total store size, whichever is smaller
+ return min(static_cast<int64_t>(maxSize()), Config.Store.maxObjectSize);
+}
+
+void
+SwapDir::maxObjectSize(int64_t newMax)
+{
+ // negative values mean no limit (-1)
+ if (newMax < 0) {
+ max_objsize = -1; // set explicitly in case it had a non-default value
previously
+ return;
+ }
+
+ // prohibit values greater than total storage area size
+ // but set max_objsize to the maximum allowed to override
maximum_object_size global config
+ if (static_cast<uint64_t>(newMax) > maxSize()) {
+ debugs(47, DBG_PARSE_NOTE(2), "WARNING: object 'max-size' option for "
<< path << " cannot exceed " << maxSize());
+ max_objsize = maxSize();
+ return;
+ }
+
+ max_objsize = newMax;
+}
+
void
SwapDir::reference(StoreEntry &) {}
=== modified file 'src/SwapDir.h'
--- src/SwapDir.h 2013-01-31 15:05:34 +0000
+++ src/SwapDir.h 2013-02-03 13:36:43 +0000
@@ -146,9 +146,15 @@
virtual uint64_t maxSize() const { return max_size;}
+ /// the minimum size of singel object which may be stored here
virtual uint64_t minSize() const;
- virtual int64_t maxObjectSize() const { return max_objsize; }
+ /// The maximum size of single object which may be stored here.
+ int64_t maxObjectSize() const;
+
+ /// configure the maximum object size for this storage area.
+ /// May be any size up to the total storage area.
+ void maxObjectSize(int64_t newMax);
virtual void getStats(StoreInfoStats &stats) const;
virtual void stat (StoreEntry &anEntry) const;
@@ -180,13 +186,13 @@
protected:
uint64_t max_size; ///< maximum allocatable size of the storage area
+ int64_t min_objsize; ///< minimum size of any object stored here (-1
for no limit)
+ int64_t max_objsize; ///< maximum size of any object stored here (-1
for no limit)
public:
char *path;
int index; /* This entry's index into the swapDirs array */
int disker; ///< disker kid id dedicated to this SwapDir or -1
- int64_t min_objsize;
- int64_t max_objsize;
RemovalPolicy *repl;
int removals;
int scanned;
=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc 2013-02-04 09:47:50 +0000
+++ src/cache_cf.cc 2013-02-05 04:08:33 +0000
@@ -277,16 +277,23 @@
static void
update_maxobjsize(void)
{
- int i;
int64_t ms = -1;
- for (i = 0; i < Config.cacheSwap.n_configured; ++i) {
+ // 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());
- if (dynamic_cast<SwapDir *>(Config.cacheSwap.swapDirs[i].getRaw())->
- max_objsize > ms)
- ms = dynamic_cast<SwapDir
*>(Config.cacheSwap.swapDirs[i].getRaw())->max_objsize;
+ 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 < Config.Store.maxInMemObjSize)
+ ms = Config.Store.maxInMemObjSize;
+
store_maxobjsize = ms;
}
=== modified file 'src/cf.data.pre'
--- src/cf.data.pre 2013-02-04 05:55:39 +0000
+++ src/cf.data.pre 2013-02-04 06:10:22 +0000
@@ -3134,7 +3134,7 @@
replacement policies.
NOTE: if using the LFUDA replacement policy you should increase
- the value of maximum_object_size above its default of 4096 KB to
+ the value of maximum_object_size above its default of 4 MB to
to maximize the potential byte hit rate improvement of LFUDA.
For more information about the GDSF and LFUDA cache replacement
@@ -3333,14 +3333,18 @@
NAME: maximum_object_size
COMMENT: (bytes)
TYPE: b_int64_t
-DEFAULT: 4096 KB
+DEFAULT: 4 MB
LOC: Config.Store.maxObjectSize
DOC_START
- Objects larger than this size will NOT be saved on disk. The
- value is specified in kilobytes, and the default is 4MB. If
- you wish to get a high BYTES hit ratio, you should probably
+ The default limit on size of objects stored to disk.
+ This size is used for cache_dir where max-size is not set.
+ The value is specified in bytes, and the default is 4 MB.
+
+ If you wish to get a high BYTES hit ratio, you should probably
increase this (one 32 MB object hit counts for 3200 10KB
- hits). If you wish to increase speed more than your want to
+ hits).
+
+ If you wish to increase hit ratio more than you want to
save bandwidth you should leave this low.
NOTE: if using the LFUDA replacement policy you should increase
=== modified file 'src/fs/rock/RockIoState.cc'
--- src/fs/rock/RockIoState.cc 2012-09-01 14:38:36 +0000
+++ src/fs/rock/RockIoState.cc 2013-02-03 13:37:43 +0000
@@ -24,7 +24,7 @@
{
e = anEntry;
// swap_filen, swap_dirn, diskOffset, and payloadEnd are set by the caller
- slotSize = dir->max_objsize;
+ slotSize = dir->maxObjectSize();
file_callback = cbFile;
callback = cbIo;
callback_data = cbdataReference(data);
=== modified file 'src/peer_digest.cc'
--- src/peer_digest.cc 2013-01-30 15:39:37 +0000
+++ src/peer_digest.cc 2013-02-03 13:38:13 +0000
@@ -347,6 +347,7 @@
req->header.putStr(HDR_ACCEPT, "text/html");
+ // XXX: this is broken when login=PASS, login=PASSTHRU, login=PROXYPASS,
login=NEGOTIATE, and login=*:...
if (p->login)
xstrncpy(req->login, p->login, MAX_LOGIN_SZ);
=== modified file 'src/store.cc'
--- src/store.cc 2013-01-14 11:44:31 +0000
+++ src/store.cc 2013-02-03 13:38:25 +0000
@@ -1013,9 +1013,8 @@
++store_check_cachable_hist.no.negative_cached;
return 0; /* avoid release call below */
} else if ((getReply()->content_length > 0 &&
- getReply()->content_length
- > Config.Store.maxObjectSize) ||
- mem_obj->endOffset() > Config.Store.maxObjectSize) {
+ getReply()->content_length > store_maxobjsize) ||
+ mem_obj->endOffset() > store_maxobjsize) {
debugs(20, 2, "StoreEntry::checkCachable: NO: too big");
++store_check_cachable_hist.no.too_big;
} else if (checkTooSmall()) {
=== modified file 'src/store_dir.cc'
--- src/store_dir.cc 2013-02-03 13:51:45 +0000
+++ src/store_dir.cc 2013-02-03 13:53:27 +0000
@@ -277,10 +277,10 @@
/* If the load is equal, then look in more details */
if (load == least_load) {
- /* closest max_objsize fit */
+ /* closest max-size fit */
if (least_objsize != -1)
- if (SD->max_objsize > least_objsize || SD->max_objsize == -1)
+ if (SD->maxObjectSize() > least_objsize)
continue;
/* most free */
@@ -289,7 +289,7 @@
}
least_load = load;
- least_objsize = SD->max_objsize;
+ least_objsize = SD->maxObjectSize();
most_free = cur_free;
dirn = i;
}
=== modified file 'src/store_swapout.cc'
--- src/store_swapout.cc 2013-02-05 01:13:29 +0000
+++ src/store_swapout.cc 2013-02-05 04:11:38 +0000
@@ -441,10 +441,11 @@
const int64_t maxKnownSize = mem_obj->availableForSwapOut();
debugs(20, 7, HERE << "maxKnownSize= " << maxKnownSize);
/*
- * NOTE: the store_maxobjsize here is the max of optional
- * max-size values from 'cache_dir' lines. It is not the
- * same as 'maximum_object_size'. By default, store_maxobjsize
- * will be set to -1. However, I am worried that this
+ * NOTE: the store_maxobjsize here is the global maximum
+ * size of object cacheable in any of Squid cache stores
+ * both disk and memory stores.
+ *
+ * However, I am worried that this
* deferance may consume a lot of memory in some cases.
* Should we add an option to limit this memory consumption?
*/
=== modified file 'src/tests/testRock.cc'
--- src/tests/testRock.cc 2013-01-28 16:56:05 +0000
+++ src/tests/testRock.cc 2013-02-05 09:13:39 +0000
@@ -71,6 +71,7 @@
strtok(config_line, w_space);
store->parse(0, path);
+ store_maxobjsize = 1024*1024*2;
safe_free(path);
@@ -179,8 +180,7 @@
StoreEntry *const pe =
storeCreateEntry(url, "dummy log url", flags, Http::METHOD_GET);
HttpReply *const rep = const_cast<HttpReply *>(pe->getReply());
- rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test",
- -1, -1, squid_curtime + 100000);
+ rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test", 0,
-1, squid_curtime + 100000);
pe->setPublicKey();
@@ -239,6 +239,8 @@
StoreEntry *const pe = addEntry(i);
+ printf("pe->swap_status == %d (SWAPOUT_WRITING=%d)\n",
pe->swap_status, SWAPOUT_WRITING);
+
CPPUNIT_ASSERT(pe->swap_status == SWAPOUT_WRITING);
CPPUNIT_ASSERT(pe->swap_dirn == 0);
CPPUNIT_ASSERT(pe->swap_filen >= 0);
=== modified file 'src/tests/testUfs.cc'
--- src/tests/testUfs.cc 2013-01-28 16:56:05 +0000
+++ src/tests/testUfs.cc 2013-02-03 13:38:39 +0000
@@ -111,6 +111,7 @@
strtok(config_line, w_space);
aStore->parse(0, path);
+ store_maxobjsize = 1024*1024*2;
safe_free(path);
@@ -145,7 +146,7 @@
flags.cachable = true;
StoreEntry *pe = storeCreateEntry("dummy url", "dummy log url", flags,
Http::METHOD_GET);
HttpReply *rep = (HttpReply *) pe->getReply(); // bypass const
- rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test",
-1, -1, squid_curtime + 100000);
+ rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test",
0, -1, squid_curtime + 100000);
pe->setPublicKey();
