Amos already +1-ed the patch, I have no insights to add so unless
someone speaks up real fast, we proceed.
On Tue, Jun 24, 2014 at 9:29 PM, Alex Rousskov
<rouss...@measurement-factory.com> wrote:
> 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
>> <rouss...@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
