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

We can't know except by hammering it.
Did you run coadvisor and polygraph against it?
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.

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.
>

-- 
    Francesco