]> git.ipfire.org Git - thirdparty/squid.git/commit - src/store_client.cc
Changes revolving around making Store work with SMP-shared max-size cache_dirs:
authorAlex Rousskov <rousskov@measurement-factory.com>
Tue, 15 Feb 2011 04:02:28 +0000 (21:02 -0700)
committerAlex Rousskov <rousskov@measurement-factory.com>
Tue, 15 Feb 2011 04:02:28 +0000 (21:02 -0700)
commitaa1a691ec62c31bd0aa3d1c46a1bf83e16cd280c
tree714898395551c58392bdbd153476e5c5cc5d57bb
parenta5c1e9415ed08608c29a6c6e966fbf7a97ac4d3c
Changes revolving around making Store work with SMP-shared max-size cache_dirs:

* Added MemObject::expectedReplySize() and used it instead of object_sz.

When deciding whether an object with a known content length can be swapped
out, do not wait until the object is completely received and its size
(mem_obj->object_sz) becomes known (while asking the store to recheck in vain
with every incoming chunk). Instead, use the known content length, if any, to
make the decision.

This optimizes the common case where the complete object is eventually
received and swapped out, preventing accumulating potentially large objects in
RAM while waiting for the end of the response. Should not affect objects with
unknown content length.

Side-effect1: probably fixes several cases of unknowingly using negative
(unknown) mem_obj->object_sz in calculations. I added a few assertions to
double check some of the remaining object_sz/objectLen() uses.

Side-effect2: When expectedReplySize() is stored on disk as StoreEntry
metadata, it may help to detect truncated entries when the writer process dies
before completing the swapout.

* Removed mem->swapout.memnode in favor of mem->swapout.queue_offset.

The code used swapout.memnode pointer to keep track of the last page that was
swapped out. The code was semi-buggy because it could reset the pointer to
NULL if no new data came in before the call to doPages(). Perhaps the code
relied on the assumption that the caller will never doPages if there is no new
data, but I am not sure that assumption was correct in all cases (it could be
that I broke the calling code, of course).

Moreover, the page pointer was kept without any protection from page
disappearing during asynchronous swapout. There were "Evil hack time" comments
discussing how the page might disappear.

Fortunately, we already have mem->swapout.queue_offset that can be fed to
getBlockContainingLocation to find the page that needs to be swapped out.
There is no need to keep the page pointer around. The queue_offset-based math
is the same so we are not adding any overheads by using that offset (in fact,
we are removing some minor computations).

* Added "close how?" parameter to storeClose() and friends.

The old code would follow the same path when closing swapout activity for an
aborted entry and when completing a perfectly healthy swapout. In non-shared
case, that could have been OK because the abort code would then release the
entry, removing any half-written entry from the index and the disk (but I am
not sure that release happened fast enough in 100% of cases).

When the index and disk storage is shared among workers, such "temporary"
inconsistencies result in truncated responses being delivered by other workers
to the user because once the swapout activity is closed, other workers can
start using the entry.

By adding the "close how?" parameter to closing methods we allow the core and
SwapDir-specific code to handle aborted swapouts appropriately.

Since swapin code is "read only", we do not currently distinguish between
aborted and fully satisfied readers: The readerGone enum value applies to both
cases. If needed, the SwapDir reading code can make that distinction by
analyzing how much was actually swapped in.

* Moved "can you store this entry?" code to virtual SwapDir::canStore().

The old code had some of the tests in SwapDir-specific canStore() methods and
some in storeDirSelect*() methods. This resulted in inconsistencies, code
duplication, and extra calculation overheads. Making this call virtual allows
individual cache_dir types to do custom access controls.

The same method is used for cache_dir load reporting (if it returns true).
Load management needs more work, but the current code is no worse than the old
one in this aspect, and further improvements are outside this change scope.

* Minimized from-disk StoreEntry loading/unpacking code duplication.

Moved common (and often rather complex!) code from store modules into
storeRebuildLoadEntry, storeRebuildParseEntry, and storeRebuildKeepEntry.

* Do not set object_sz when the entry is aborted because the true object size
(HTTP reply headers + body) is not known in this case. Setting object_sz may
fool client-side code into believing that the object is complete.

This addresses an old RBC's complaint.

* When swapout initiation fails, release StoreEntry. This prevents the caller
code from trying to swap out again and again because swap_status becomes
SWAPOUT_NONE.

TODO: Consider add SWAPOUT_ERROR, STORE_ERROR, and similar states. It may
solve several problems where the code sees _NONE or _OK and thinks everything
is peachy when in fact there was an error.

* Always call StoreEntry::abort() instead of setting ENTRY_ABORTED manually.

* Rely on entry->abort() side-effects if ENTRY_ABORTED was set.

* Added or updated comments to better document current code.

* Added operator << for dumping StoreEntry summary into the debugging log.
Needs more work to report more info (and not report yet-unknown info).
26 files changed:
src/MemObject.cc
src/MemObject.h
src/Store.h
src/StoreIOState.h
src/StoreMeta.h
src/SwapDir.cc
src/SwapDir.h
src/client_side_reply.cc
src/fs/coss/CossSwapDir.h
src/fs/coss/store_dir_coss.cc
src/fs/ufs/store_dir_ufs.cc
src/fs/ufs/store_io_ufs.cc
src/fs/ufs/ufscommon.cc
src/fs/ufs/ufscommon.h
src/protos.h
src/store.cc
src/store_client.cc
src/store_dir.cc
src/store_io.cc
src/store_rebuild.cc
src/store_swapmeta.cc
src/store_swapout.cc
src/tests/TestSwapDir.cc
src/tests/TestSwapDir.h
src/tests/stub_store_rebuild.cc
src/tests/stub_store_swapout.cc