Re: Caching responses with "Vary" responses

On Wed, 2012-07-18 at 11:10 -0600, Alex Rousskov wrote:
> On 07/17/2012 09:28 AM, Alexander Komyagin wrote:
>
> > 2) Second one: even when we call startWriting() for the correct store
> > key (which one is the same store hash as an response for the same URL
> > without Vary - thanks to Henrik for the explanation!), our StoreEntry
> > won't get swapped: after startWriting() the writeComplete() routine is
> > called, which calls invokeHandlers() and, eventually, swapOut().
> > In swapOut() after checking that mayStartSwapOut() is false (complete()
> > was not called yet, so no content length is set), we are calling
> > trimMemory(), which sees our StoreEntry as a subject for trimming - so
> > it releases the request and makes our entry private - this indeed
> > prevents the entry to be swapped.
>
> Does the above apply to trunk code after revision 12205 which attempts
> to fix how memory trimming works? If not, please re-evaluate whether the
> problem is still there.

I've checked HEAD-BZR and it seems that problem 2 is fixed - memory
trimming does not cause problems anymore (thanks to maybeTrimMemory,
though it obscures the logic a bit IMHO),
but problem 1 (saving to wrong store key) and shm saving bug are still
there.

Please use attached squidv3-vary-cache-1.patch as a solution for problem
#1.

>
>
> > It seems that on the first HIT we have successfully loaded data from the
> > swap store, but then we save the data to shared memory. So the next
> > request takes data from shm. And it looks like Squid doesn't save
> > mem_obj->vary_headers to shm!!
>
> Shared memory cache is meant to be just like a disk cache in this
> aspect. It does not know anything about mem_obj->vary_headers and just
> packs/unpacks the STORE_META_STD TLV (see StoreMapSlot
> src/ipc/StoreMap.h) part of StoreEntry.
>
> I do not see vary_headers used by any src/fs/* code. Do you know how the
> disk cache gets them? Perhaps there is a higher level call to pack and
> unpack Vary request headers that the MemStore is missing. We can add
> special field/code to StoreMapSlot but I would prefer to see how it is
> done for the disk storage first, in hope that we can reuse the same code.
>

For the disc cache, STORE_META_VARY_HEADERS is used (see
store_swapmeta.cc). Appropriate tlv is added to the list.

But for shm I don't see any SwapMeta work! There are some calls that are
writing reply headers and body to the internal mem_hdr object and then I
can see this object data copied to shm!

In the meantime, those who are in need can use my small dirty hack
(squidv3-vary-headers-shm-hack.patch) that packs mem_obj->vary_headers
as an additional reply header, so it can be flawlessly saved to shm and
recovered afterwards.

>
> Thank you,
>
> Alex.
>
>
>
> > On Fri, 2012-07-13 at 12:21 +0400, Alexander Komyagin wrote:
> >> OK. So, regarding the log I've previously attached, those 235 bytes
> >> (vary header) shall be saved on C447BB767EC4BC4DAC1820CE7999F4C4, while
> >> the response itself (7808 bytes) can be saved on any other, right?
> >>
> >> If it's right, we got two problems here:
> >> 1) Vary header hash is not correct (shall be
> >> C447BB767EC4BC4DAC1820CE7999F4C4, but got
> >> 4DB6DCFE80050CF0CA306DB8EDFF718E)
> >> 2) Vary header doesn't get saved to the store
> >> ( StoreEntry::checkCachable: NO: private key)
> >>
> >> On Thu, 2012-07-12 at 22:29 +0200, Henrik Nordström wrote:
> >>> tor 2012-07-12 klockan 14:32 +0400 skrev Alexander Komyagin:
> >>>
> >>>> 2) What are those 235 bytes that are written to the 'vary' storeEntry
> >>>> (created in setPublicKey)? And what is the purpose of that additional
> >>>> storeEntry?
> >>>
> >>> It's the Vary header of the response so future lookups know how to build
> >>> the store key.
> >>>
> >>> This object should get saved on the same store hash as an response for
> >>> the same URL without Vary.
> >>>
> >>> Regards
> >>> Henrik
> >>>
> >>
> >
>
>

-- 
Best wishes,
Alexander Komyagin