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.
