Re: not caching ecap munged body

Oops, here's the diff from Alex which I forgot to include below:

=== modified file 'src/adaptation/ecap/MessageRep.cc'
--- src/adaptation/ecap/MessageRep.cc 2010-05-26 03:06:02 +0000
+++ src/adaptation/ecap/MessageRep.cc 2011-01-26 21:43:36 +0000
@@ -44,24 +44,30 @@
 void
 Adaptation::Ecap::HeaderRep::add(const Name &name, const Value &value)
 {
     const http_hdr_type squidId = TranslateHeaderId(name); // HDR_OTHER OK
     HttpHeaderEntry *e = new HttpHeaderEntry(squidId, name.image().c_str(),
             value.toString().c_str());
     theHeader.addEntry(e);
+
+ // XXX: this is too often and not enough, on many levels
+ theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
 }

 void
 Adaptation::Ecap::HeaderRep::removeAny(const Name &name)
 {
     const http_hdr_type squidId = TranslateHeaderId(name);
     if (squidId == HDR_OTHER)
         theHeader.delByName(name.image().c_str());
     else
         theHeader.delById(squidId);
+
+ // XXX: this is too often and not enough, on many levels
+ theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
 }

 libecap::Area
 Adaptation::Ecap::HeaderRep::image() const
 {
     MemBuf mb;
     mb.init();

This only works, I think, because when removing Content-Length we end up with a -1 in the length for the whole message, instead of the old (too large) content length. (There's a Adaptation::Ecap::XactionRep::setAdaptedBodySize function that's commented out in the eCAP support, presumably because it's common to not know the length of the final adapted body, like in the gzip case.)

There are a few options - assume the content-length for adapted messages is just the total object_len - hdr_sz, leave it always -1 unless the eCAP module tells you otherwise by setting the Content-Length header maybe, or finally, store it by accumulating the size of returned chunks along the way receiving the adapted body and set it accordingly when there's no more adapted body to receive.

Any suggestions for the best route to pursue?

Regards,
Jonathan Wolfe

On Jan 26, 2011, at 2:35 PM, Jonathan Wolfe wrote:

> Okay, I narrowed this down a bit more with some help from Alex Rousskov:
>
> When it works (doing a string replace from "asdf" to "fdsa" for example, so same total content length):
>
> 2011/01/26 16:07:21.312| storeEntryValidLength: Checking '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 16:07:21.312| storeEntryValidLength: object_len = 20366
> 2011/01/26 16:07:21.312| storeEntryValidLength: hdr_sz = 360
> 2011/01/26 16:07:21.312| storeEntryValidLength: content_length = 20006
> 2011/01/26 16:07:21.317| StoreEntry::setMemStatus: inserted mem node http://www.example.com/squid-test
>
> When it doesn't work ("asdf" to just "a"):
>
> 2011/01/26 16:05:59.878| storeEntryValidLength: Checking '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 16:05:59.878| storeEntryValidLength: object_len = 14843
> 2011/01/26 16:05:59.878| storeEntryValidLength: hdr_sz = 360
> 2011/01/26 16:05:59.878| storeEntryValidLength: content_length = 20006
> 2011/01/26 16:05:59.878| storeEntryValidLength: 5523 bytes too small; '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 16:05:59.879| StoreEntry::checkCachable: NO: wrong content-length
>
> The headers returned in both cases don't actually include a Content-Length header, which is removed by the module using adapted->header().removeAny.
>
> It looks like squid is restoring the content length in the second case, and declaring it too small.
>
> See https://answers.launchpad.net/ecap/+question/142965 for my discussion with Alex on this. The diff he provided, which is repeated here, seems to have the effect of setting the message content length to -1 when removing the content length header from within the ecap module, and that results in this:
>
> 2011/01/26 17:21:46.539| storeEntryValidLength: Checking '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 17:21:46.539| storeEntryValidLength: object_len = 16190
> 2011/01/26 17:21:46.539| storeEntryValidLength: hdr_sz = 360
> 2011/01/26 17:21:46.539| storeEntryValidLength: content_length = -1
> 2011/01/26 17:21:46.539| storeEntryValidLength: Unspecified content length: 1078B4E8EC1D17CFEBCD533EE19F7FD6
> 2011/01/26 17:21:46.544| StoreEntry::setMemStatus: inserted mem node http://www.example.com/squid-test
>
> Not the best behavior, but it does cache as expected now.
>
> Likely there's a better place to reset the content length, right? Perhaps in src/adaptation/ecap/XactionRep.cc, in moveAbContent() when we've received the full adapted body?
>
> Regards,
> -Jon
>
> On Jan 23, 2011, at 8:46 PM, Amos Jeffries wrote:
>
>> On 24/01/11 13:43, Henrik Nordström wrote:
>>> lör 2011-01-22 klockan 23:04 +1300 skrev Amos Jeffries:
>>>
>>>> Squid caches only one of N variants so the expected behviour is that
>>>> each new variant is a MISS but becomes a HIT on repeated duplicate
>>>> requests until a new variant pushes it out of cache.
>>>
>>> No it caches all N variants seen if the origin response has Vary:
>>>
>>> But not sure what happens with the gzip eCAP module in this regard.
>>
>> AFAIK, that proper variant handling was not yet ported to squid-3. Only in squid-2 right now.
>> This identical behaviour is causing some problems with recent Chrome using sdch encoding. Thus clashing with the gzip|deflate cached variant from other browsers.
>>
>> Though yes the adapter output seems to be borked anyway.
>>
>> Amos
>> --
>> Please be using
>> Current Stable Squid 2.7.STABLE9 or 3.1.10
>> Beta testers wanted for 3.2.0.4
>
Received on Thu Feb 03 2011 - 04:09:19 MST