Re: [PATCH] Fixes to get more collapsed forwarding hits

On 06/24/2014 12:55 PM, Kinkie wrote:

> Did you run coadvisor and polygraph against it?

Co-Advisor does not have collapsed forwarding cases (there are no
explicit RFC 2616 MUSTs that cover CF although some cases can be
certainly added to test some MUSTs in a CF context).

Polygraph can be used to simulate CF-heavy environment, but all you will
get from this patch is an improved hit ratio. It is not going to tell
you which collapsing cases Squid have missed.

> If not, and if the branch is public, it's trivial tor un them against
> it now (thanks TMF, thanks Pavel!). It doesn't guarantee to catch all
> cases, but at least it should raise confidence.

There is currently no branch for this -- just a trunk patch. A bigger
problem is that you will need a different Polygraph workload -- the one
we use for Squid regression testing via Jenkins is unlikely to produce a
lot of CF opportunities.

Overall, while I would certainly welcome Co-Advisor and Polygraph
results, I decided to rely on existing trunk regression testing and not
spend more time on organizing custom pre-commit tests. If somebody wants
to volunteer to test this, please let me know.

Thank you,

Alex.

> On Tue, Jun 24, 2014 at 7:37 PM, Alex Rousskov
> <rousskov_at_measurement-factory.com> wrote:
>> On 06/23/2014 11:58 PM, Amos Jeffries wrote:
>>> On 24/06/2014 4:07 p.m., Alex Rousskov wrote:
>>>> * Do not abandon writing a collapsed cache entry when we cannot cache
>>>> the entry in RAM if the entry can be cached on disk instead. Both shared
>>>> memory cache and the disk cache have to refuse to cache the entry for it
>>>> to become non-collapsible. This dual refusal is difficult to detect
>>>> because each cache may make the caching decision at different times.
>>>> Added StoreEntry methods to track those decisions and react to them.
>>
>>
>>> Hmm. FTR: IMHO the only reasons a response should be non-collapsible is
>>> if it is a) private, b) authenticated to another user, c) explicitly
>>> known non-cacheable, d) a non-matching variant of the URL, or e) we
>>> already discarded some body bytes.
>>
>> Yes, there are many "only" reasons why a we may not be able to collapse
>> what looked like a collapsible transaction at the beginning. The bullet
>> we are discussing fixes a case where the Squid shared memory caching
>> code said "if we cannot cache this entry in the shared memory cache,
>> then we cannot collapse onto this entry". That was wrong; we should
>> still be able to collapse if the disk cache stores the entry.
>>
>>
>>> The factor of memory/disk caches not having provided cacheable size
>>> allow/reject result yet is no reason to be non-collapsible. It is a
>>> reason to wait on that result to determine the cacheability condition (c).
>>
>> Yes, kind of. Today, Squid treats "HTTP cachable" and "storable in a
>> Squid cache" as two rather different things. The two conditions are
>> checked and managed in different code places and at different times
>> (with some overlap). And some of those decisions are unrelated to the
>> entry size. However, they are essentially the same from user-perceived
>> caching point of view and can be listed under a single "explicitly known
>> non-cacheable" bullet if one is careful to interpret that correctly.
>>
>>
>>> +1. Although I do not know enough about store yet to do much in-depth
>>> audit of the logic changes.
>>
>> FWIW, knowing "enough" about Store makes me even less confident about
>> changes like these ones. This is very tricky stuff not just because of
>> the complexity of the features involved (caching, collapsing, etc.) but
>> because there are very few simple invariants in Store. I rarely know for
>> sure what I am going to break when changing this stuff. The code is
>> getting better, and I am trying to document the invariants I find, but
>> it is a very slow process, and there will be many more bugs, possibly
>> from this patch as well.
>>
>>
>> Cheers,
>>
>> Alex.
>>
>
>
>
Received on Tue Jun 24 2014 - 19:29:20 MDT