On Sun, 15 May 2011 15:11:08 -0600, Alex Rousskov wrote:
Hello,
The attached patch contains the first part of the SMP caching
changes described in the "SMP Rock Store and shared memory cache
patches" email. I labeled it [PREVIEW] because of the following
outstanding TODO items:
* Portability tests and changes (via Hudson).
* Add mem_cache shared=on|off parameter.
However, I do not expect the above items to cause significant changes
to
the patch.
The patch preamble contains a detailed change log.
For the complete set of changes (all three parts), and for testing,
please see the following Launchpad branch:
lp:~measurement-factory/squid/3p2-rock
Thank you,
Alex.
Just done a quick read-through...
includes/Array.h
- can go in separately and immediately if you wish.
src/adaptation/icap/*
- changes appear to be relatively separate. I suspect that RST
handling may be of additional use with some 3.1 error recovery. If you
agree that can go in separately and let me know to port it back for use
there.
src/cf.data.pre:
- you are adding documentation for cache_dir "rock" type. That should
be in the other patch where rock is actually added.
- contains documentation for logformat width changes. Please remove
from this patch. AFAIK the width changes are still waiting on a separate
commit to happen. If that was already committed, please point me at the
trunk revno and apply this extra documentation fix separately so I can
group it with the relevant feature patch set.
- contains documentation from "Dynamic adaptation plan over multiple
vectoring points".
Please remove and apply separately if still relevant, so it too can
be grouped properly to its feature patch set.
src/base/RunnersRegistry.*:
- what are runners? please document a bit clearer than "different
modules".
src/store_rebuild.cc:
- Please convert the debugs() level-1 to DBG_IMPORTANT while moving
the code to storeRebuildLoadEntry() and storeRebuildParseEntry().
(search for ", 1," to find other places in the patch needing same)
- Also instead of using HERE they need "WARNING:" prefix and a suffix
statement that Squid will erase the broken or collision object from
cache.
src/main.cc:
- chunk line 1232 has similar debugs() issues missing DBG_CRITICAL and
"FATAL:" prefix in the new version added.
In StoreEntry::swapoutPossible() the test for "if (expectedEnd < 0)"
looks out of place. IMO the current-min > global-max should go above it
to short-cut very large unknown-length objects repeating the swapout
tests when they grow bigger than cacheable. Please confirm that.
Amos
