diff -ru squid-3.2.0.14/src/MemObject.cc squid-3.2.0.14-mem_node/src/MemObject.cc
--- squid-3.2.0.14/src/MemObject.cc 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/MemObject.cc 2012-01-17 20:11:54.756609310 +0100
@@ -492,3 +492,9 @@
}
#endif
+
+int64_t
+MemObject::availableForSwapOut() const
+{
+ return endOffset() - swapout.queue_offset;
+}
diff -ru squid-3.2.0.14/src/MemObject.h squid-3.2.0.14-mem_node/src/MemObject.h
--- squid-3.2.0.14/src/MemObject.h 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/MemObject.h 2012-01-17 20:11:54.757609437 +0100
@@ -81,6 +81,7 @@
*/
int64_t objectBytesOnDisk() const;
int64_t policyLowestOffsetToKeep(bool swap) const;
+ int64_t availableForSwapOut() const; ///< buffered bytes we have not swapped out yet
void trimSwappable();
void trimUnSwappable();
bool isContiguous() const;
diff -ru squid-3.2.0.14/src/store.cc squid-3.2.0.14-mem_node/src/store.cc
--- squid-3.2.0.14/src/store.cc 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/store.cc 2012-01-17 20:11:54.761609945 +0100
@@ -1890,95 +1890,8 @@
return result;
}
-bool
-StoreEntry::swapoutPossible()
-{
- if (!Config.cacheSwap.n_configured)
- return false;
-
- /* should we swap something out to disk? */
- debugs(20, 7, "storeSwapOut: " << url());
- debugs(20, 7, "storeSwapOut: store_status = " << storeStatusStr[store_status]);
-
- assert(mem_obj);
- MemObject::SwapOut::Decision &decision = mem_obj->swapout.decision;
-
- // if we decided that swapout is not possible, do not repeat same checks
- if (decision == MemObject::SwapOut::swImpossible) {
- debugs(20, 3, "storeSwapOut: already rejected");
- return false;
- }
-
- // this flag may change so we must check it even if we already said "yes"
- if (EBIT_TEST(flags, ENTRY_ABORTED)) {
- assert(EBIT_TEST(flags, RELEASE_REQUEST));
- // StoreEntry::abort() already closed the swap out file, if any
- decision = MemObject::SwapOut::swImpossible;
- return false;
- }
-
- // if we decided that swapout is possible, do not repeat same checks
- if (decision == MemObject::SwapOut::swPossible) {
- debugs(20, 3, "storeSwapOut: already allowed");
- return true;
- }
-
- // if we are swapping out already, do not repeat same checks
- if (swap_status != SWAPOUT_NONE) {
- debugs(20, 3, "storeSwapOut: already started");
- decision = MemObject::SwapOut::swPossible;
- return true;
- }
-
- if (!checkCachable()) {
- debugs(20, 3, "storeSwapOut: not cachable");
- decision = MemObject::SwapOut::swImpossible;
- return false;
- }
-
- if (EBIT_TEST(flags, ENTRY_SPECIAL)) {
- debugs(20, 3, "storeSwapOut: " << url() << " SPECIAL");
- decision = MemObject::SwapOut::swImpossible;
- return false;
- }
-
- // check cache_dir max-size limit if all cache_dirs have it
- if (store_maxobjsize >= 0) {
- // TODO: add estimated store metadata size to be conservative
-
- // use guaranteed maximum if it is known
- const int64_t expectedEnd = mem_obj->expectedReplySize();
- debugs(20, 7, "storeSwapOut: expectedEnd = " << expectedEnd);
- if (expectedEnd > store_maxobjsize) {
- debugs(20, 3, "storeSwapOut: will not fit: " << expectedEnd <<
- " > " << store_maxobjsize);
- decision = MemObject::SwapOut::swImpossible;
- return false; // known to outgrow the limit eventually
- }
-
- // use current minimum (always known)
- const int64_t currentEnd = mem_obj->endOffset();
- if (currentEnd > store_maxobjsize) {
- debugs(20, 3, "storeSwapOut: does not fit: " << currentEnd <<
- " > " << store_maxobjsize);
- decision = MemObject::SwapOut::swImpossible;
- return false; // already does not fit and may only get bigger
- }
-
- // prevent default swPossible answer for yet unknown length
- if (expectedEnd < 0) {
- debugs(20, 3, "storeSwapOut: wait for more info: " <<
- store_maxobjsize);
- return false; // may fit later, but will be rejected now
- }
- }
-
- decision = MemObject::SwapOut::swPossible;
- return true;
-}
-
void
-StoreEntry::trimMemory()
+StoreEntry::trimMemory(const bool preserveSwappable)
{
/*
* DPW 2007-05-09
@@ -1988,7 +1901,7 @@
if (mem_status == IN_MEMORY)
return;
- if (!swapOutAble()) {
+ if (!preserveSwappable) {
if (mem_obj->policyLowestOffsetToKeep(0) == 0) {
/* Nothing to do */
return;
Endast i squid-3.2.0.14-mem_node/src: store.cc.orig
diff -ru squid-3.2.0.14/src/store_client.cc squid-3.2.0.14-mem_node/src/store_client.cc
--- squid-3.2.0.14/src/store_client.cc 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/store_client.cc 2012-01-17 20:11:54.768610834 +0100
@@ -194,7 +194,7 @@
if (getType() == STORE_DISK_CLIENT)
/* assert we'll be able to get the data we want */
/* maybe we should open swapin_sio here */
- assert(entry->swap_filen > -1 || entry->swapOutAble());
+ assert(entry->swap_filen > -1 || entry->swappingOut());
#if STORE_CLIENT_LIST_DEBUG
Endast i squid-3.2.0.14-mem_node/src: store_client.cc.orig
diff -ru squid-3.2.0.14/src/Store.h squid-3.2.0.14-mem_node/src/Store.h
--- squid-3.2.0.14/src/Store.h 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/Store.h 2012-01-17 20:11:56.770865312 +0100
@@ -92,8 +92,9 @@
virtual char const *getSerialisedMetaData();
void replaceHttpReply(HttpReply *, bool andStartWriting = true);
void startWriting(); ///< pack and write reply headers and, maybe, body
- virtual bool swapoutPossible();
- virtual void trimMemory();
+ /// whether we may start writing to disk (now or in the future)
+ virtual bool mayStartSwapOut();
+ virtual void trimMemory(const bool preserveSwappable);
void abort();
void unlink();
void makePublic();
@@ -108,7 +109,8 @@
void purgeMem();
void cacheInMemory(); ///< start or continue storing in memory cache
void swapOut();
- bool swapOutAble() const;
+ /// whether we are in the process of writing this entry to disk
+ bool swappingOut() const { return swap_status == SWAPOUT_WRITING; }
void swapOutFileClose(int how);
const char *url() const;
int checkCachable();
@@ -247,9 +249,9 @@
store_client_t storeClientType() const {return STORE_MEM_CLIENT;}
char const *getSerialisedMetaData();
- bool swapoutPossible() {return false;}
+ bool mayStartSwapout() {return false;}
- void trimMemory() {}
+ void trimMemory(const bool preserveSwappable) {}
static NullStoreEntry _instance;
diff -ru squid-3.2.0.14/src/store_swapout.cc squid-3.2.0.14-mem_node/src/store_swapout.cc
--- squid-3.2.0.14/src/store_swapout.cc 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/store_swapout.cc 2012-01-17 20:11:54.771611216 +0100
@@ -187,8 +187,20 @@
if (!mem_obj)
return;
- if (!swapoutPossible())
+ // this flag may change so we must check even if we are swappingOut
+ if (EBIT_TEST(flags, ENTRY_ABORTED)) {
+ assert(EBIT_TEST(flags, RELEASE_REQUEST));
+ // StoreEntry::abort() already closed the swap out file, if any
+ // no trimming: data producer must stop production if ENTRY_ABORTED
return;
+ }
+
+ const bool weAreOrMayBeSwappingOut = swappingOut() || mayStartSwapOut();
+
+ trimMemory(weAreOrMayBeSwappingOut);
+
+ if (!weAreOrMayBeSwappingOut)
+ return; // nothing else to do
// Aborted entries have STORE_OK, but swapoutPossible rejects them. Thus,
// store_status == STORE_OK below means we got everything we wanted.
@@ -200,39 +212,10 @@
if (mem_obj->swapout.sio != NULL)
debugs(20, 7, "storeSwapOut: storeOffset() = " << mem_obj->swapout.sio->offset() );
- // buffered bytes we have not swapped out yet
- int64_t swapout_maxsize = mem_obj->endOffset() - mem_obj->swapout.queue_offset;
-
- assert(swapout_maxsize >= 0);
-
int64_t const lowest_offset = mem_obj->lowestMemReaderOffset();
debugs(20, 7, HERE << "storeSwapOut: lowest_offset = " << lowest_offset);
- // Check to see whether we're going to defer the swapout based upon size
- if (store_status != STORE_OK) {
- const int64_t expectedSize = mem_obj->expectedReplySize();
- const int64_t maxKnownSize = expectedSize < 0 ?
- swapout_maxsize : expectedSize;
- debugs(20, 7, HERE << "storeSwapOut: maxKnownSize= " << maxKnownSize);
-
- if (maxKnownSize < store_maxobjsize) {
- /*
- * 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
- * deferance may consume a lot of memory in some cases.
- * Should we add an option to limit this memory consumption?
- */
- debugs(20, 5, "storeSwapOut: Deferring swapout start for " <<
- (store_maxobjsize - maxKnownSize) << " bytes");
- return;
- }
- }
-
-// TODO: it is better to trim as soon as we swap something out, not before
- trimMemory();
#if SIZEOF_OFF_T <= 4
if (mem_obj->endOffset() > 0x7FFF0000) {
@@ -245,9 +228,9 @@
if (swap_status == SWAPOUT_WRITING)
assert(mem_obj->inmem_lo <= mem_obj->objectBytesOnDisk() );
- if (!swapOutAble())
- return;
-
+ // buffered bytes we have not swapped out yet
+ const int64_t swapout_maxsize = mem_obj->availableForSwapOut();
+ assert(swapout_maxsize >= 0);
debugs(20, 7, "storeSwapOut: swapout_size = " << swapout_maxsize);
if (swapout_maxsize == 0) { // swapped everything we got
@@ -373,19 +356,106 @@
e->unlock();
}
-/*
- * Is this entry a candidate for writing to disk?
- */
bool
-StoreEntry::swapOutAble() const
+StoreEntry::mayStartSwapOut()
{
dlink_node *node;
- if (mem_obj->swapout.sio != NULL)
+ // must be checked in the caller
+ assert(!EBIT_TEST(flags, ENTRY_ABORTED));
+
+ if (!Config.cacheSwap.n_configured)
+ return false;
+
+ assert(mem_obj);
+ MemObject::SwapOut::Decision &decision = mem_obj->swapout.decision;
+
+ // if we decided that swapout is not possible, do not repeat same checks
+ if (decision == MemObject::SwapOut::swImpossible) {
+ debugs(20, 3, HERE << " already rejected");
+ return false;
+ }
+
+ // if we decided that swapout is possible, do not repeat same checks
+ if (decision == MemObject::SwapOut::swPossible) {
+ debugs(20, 3, HERE << "already allowed");
+ return true;
+ }
+
+ // if we are swapping out already, do not repeat same checks
+ if (swap_status != SWAPOUT_NONE) {
+ debugs(20, 3, HERE << " already started");
+ decision = MemObject::SwapOut::swPossible;
return true;
+ }
- if (mem_obj->inmem_lo > 0)
+ if (!checkCachable()) {
+ debugs(20, 3, HERE << "not cachable");
+ decision = MemObject::SwapOut::swImpossible;
return false;
+ }
+
+ if (EBIT_TEST(flags, ENTRY_SPECIAL)) {
+ debugs(20, 3, HERE << url() << " SPECIAL");
+ decision = MemObject::SwapOut::swImpossible;
+ return false;
+ }
+
+ // check cache_dir max-size limit if all cache_dirs have it
+ if (store_maxobjsize >= 0) {
+ // TODO: add estimated store metadata size to be conservative
+
+ // use guaranteed maximum if it is known
+ const int64_t expectedEnd = mem_obj->expectedReplySize();
+ debugs(20, 7, HERE << "expectedEnd = " << expectedEnd);
+ if (expectedEnd > store_maxobjsize) {
+ debugs(20, 3, HERE << "will not fit: " << expectedEnd <<
+ " > " << store_maxobjsize);
+ decision = MemObject::SwapOut::swImpossible;
+ return false; // known to outgrow the limit eventually
+ }
+
+ // use current minimum (always known)
+ const int64_t currentEnd = mem_obj->endOffset();
+ if (currentEnd > store_maxobjsize) {
+ debugs(20, 3, HERE << "does not fit: " << currentEnd <<
+ " > " << store_maxobjsize);
+ decision = MemObject::SwapOut::swImpossible;
+ return false; // already does not fit and may only get bigger
+ }
+
+ // prevent default swPossible answer for yet unknown length
+ if (expectedEnd < 0) {
+ debugs(20, 3, HERE << "wait for more info: " <<
+ store_maxobjsize);
+ return false; // may fit later, but will be rejected now
+ }
+
+ if (store_status != STORE_OK) {
+ const int64_t maxKnownSize = expectedEnd < 0 ?
+ mem_obj->availableForSwapOut() : expectedEnd;
+ debugs(20, 7, HERE << "maxKnownSize= " << maxKnownSize);
+ if (maxKnownSize < store_maxobjsize) {
+ /*
+ * 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
+ * deferance may consume a lot of memory in some cases.
+ * Should we add an option to limit this memory consumption?
+ */
+ debugs(20, 5, HERE << "Deferring swapout start for " <<
+ (store_maxobjsize - maxKnownSize) << " bytes");
+ return false;
+ }
+ }
+ }
+
+ if (mem_obj->inmem_lo > 0) {
+ debugs(20, 3, "storeSwapOut: (inmem_lo > 0) imem_lo:" << mem_obj->inmem_lo);
+ decision = MemObject::SwapOut::swImpossible;
+ return false;
+ }
/*
* If there are DISK clients, we must write to disk
@@ -394,21 +464,29 @@
* therefore this should be an assert?
* RBC 20030708: We can use disk to avoid mem races, so this shouldn't be
* an assert.
+ *
+ * XXX: Not clear what "mem races" the above refers to, especially when
+ * dealing with non-cachable objects that cannot have multiple clients.
+ *
+ * XXX: If STORE_DISK_CLIENT needs SwapOut::swPossible, we have to check
+ * for that flag earlier, but forcing swapping may contradict max-size or
+ * other swapability restrictions. Change storeClientType() and/or its
+ * callers to take swap-in availability into account.
*/
for (node = mem_obj->clients.head; node; node = node->next) {
- if (((store_client *) node->data)->getType() == STORE_DISK_CLIENT)
+ if (((store_client *) node->data)->getType() == STORE_DISK_CLIENT) {
+ debugs(20, 3, HERE << "DISK client found");
+ decision = MemObject::SwapOut::swPossible;
return true;
+ }
}
- /* Don't pollute the disk with icons and other special entries */
- if (EBIT_TEST(flags, ENTRY_SPECIAL))
- return false;
-
- if (!EBIT_TEST(flags, ENTRY_CACHABLE))
- return false;
-
- if (!mem_obj->isContiguous())
+ if (!mem_obj->isContiguous()) {
+ debugs(20, 3, "storeSwapOut: not Contiguous");
+ decision = MemObject::SwapOut::swImpossible;
return false;
+ }
+ decision = MemObject::SwapOut::swPossible;
return true;
}
Endast i squid-3.2.0.14-mem_node/src: store_swapout.cc.orig
diff -ru squid-3.2.0.14/src/tests/stub_MemObject.cc squid-3.2.0.14-mem_node/src/tests/stub_MemObject.cc
--- squid-3.2.0.14/src/tests/stub_MemObject.cc 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/tests/stub_MemObject.cc 2012-01-17 20:11:54.772611344 +0100
@@ -47,6 +47,7 @@
{
return data_hdr.endOffset();
}
+int64_t MemObject::availableForSwapOut() const STUB_RETVAL(0)
void
MemObject::trimSwappable()
Endast i squid-3.2.0.14-mem_node/src/tests: stub_MemObject.cc.orig
diff -ru squid-3.2.0.14/src/tests/stub_store.cc squid-3.2.0.14-mem_node/src/tests/stub_store.cc
--- squid-3.2.0.14/src/tests/stub_store.cc 2011-12-12 12:08:18.000000000 +0100
+++ squid-3.2.0.14-mem_node/src/tests/stub_store.cc 2012-01-17 20:11:54.773611472 +0100
@@ -26,8 +26,8 @@
store_client_t StoreEntry::storeClientType() const STUB_RETVAL(STORE_NON_CLIENT)
char const *StoreEntry::getSerialisedMetaData() STUB_RETVAL(NULL)
void StoreEntry::replaceHttpReply(HttpReply *, bool andStartWriting) STUB
-bool StoreEntry::swapoutPossible() STUB_RETVAL(false)
-void StoreEntry::trimMemory() STUB
+bool StoreEntry::mayStartSwapOut() STUB_RETVAL(false)
+void StoreEntry::trimMemory(const bool preserveSwappable) STUB
void StoreEntry::abort() STUB
void StoreEntry::unlink() STUB
void StoreEntry::makePublic() STUB
@@ -41,7 +41,6 @@
void StoreEntry::invokeHandlers() STUB
void StoreEntry::purgeMem() STUB
void StoreEntry::swapOut() STUB
-bool StoreEntry::swapOutAble() const STUB_RETVAL(false)
void StoreEntry::swapOutFileClose(int how) STUB
const char *StoreEntry::url() const STUB_RETVAL(NULL)
int StoreEntry::checkCachable() STUB_RETVAL(0)