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