On 24/06/2014 4:07 p.m., Alex Rousskov wrote: > Hello, > > The attached patch contains several changes to improve the > probability of getting a collapsed forwarding hit: > > * Allow STORE_MEMORY_CLIENTs to open disk files if needed and possible. > STORE_*_CLIENT designation is rather buggy (several known XXXs). Some > collapsed clients are marked as STORE_MEMORY_CLIENTs (for the lack of > info at determination time) but their hit content may actually come from > a disk cache. > > * 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. 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). > > * Recognize disk cache as a potential source of the collapsed entry when > the memory cache is configured. While collapsed entries would normally > be found in the shared memory cache, caching policies and other factors > may prohibit memory caching but still allow disk caching. Memory cache > is still preferred. > > * Do not use unknown entry size in StoreEntry::checkTooSmall() > determination. The size of collapsed entries is often unknown, even when > they are STORE_OK (because swap_hdr_sz is unknown when the other worker > has created the cache entry). The same code has been using this > ignore-unknowns logic for the Content-Length header value, so the > rejection of unknown entry size (added as a part of C++ conversion > without a dedicated message in r5766) could have been a typo. > > > If approved, I will commit the change for the last bullet separately > because it is easier to isolate and is less specific to collapsed > forwarding. > > > The patch does not attempt to cover all possible collapsing cases that > Squid currently misses, of course. More work remains in this area > (including a serious STORE_*_CLIENT redesign), but the proposed fixes > should make a dent. > > These changes were developed for an earlier trunk version but the > trunk-ported patch posted here also passed simple lab tests. > > > Thank you, > > Alex. > +1. Although I do not know enough about store yet to do much in-depth audit of the logic changes. Amos