Re: [PATCH] reload into If-None-Match

On 07/12/2013 10:48 AM, Amos Jeffries wrote:
> On 13/07/2013 12:31 a.m., Tsantilas Christos wrote:
>> Finally I did not apply the last patch I posted because I found a bug.
>> Squid hits an assertion in clientReplyContext::processExpired at
>> client_side_reply.cc:245,
>> assert(http->storeEntry()->lastmod >= 0);
>>
>> Moreover looking in the code I am seeing that objects which does not
>> have a "Last-Modified" header are never stored in squid cache.
>> So in the patch I am posting here I removed the part of the patch which
>> allowed to use a stored entry which does not have Last-Modified header
>> but have a strong etag. This is the part which causes the assertion .
>> Alternatively we can just remove the assertion.
>>
>> However still I do not know how an entry with lastmod=-1 (no
>> Last-Modified header) found in my cache and causes the above assertion.
>>
>> Any opinion on this?

> I expect it will be the same way the no-Date objects find their way in.
> By being stored there temporarily while in-use by a client then RELEASEd
> only after completion.

If the above does happen, it means that the cache admission control code
is broken: There is no point in adding something to a cache, even
temporary, that we do not consider cachable when the response headers
are received. Multiple clients would not be able to feed off that
[temporary] cached object, and the primary client itself does not need a
cache to receive the response.

I am not saying this does not happen though (I do not know).

As for removing the assertion, I think that would be the right thing to
do long-term (why should Squid die if it finds a malformed cache
entry?), but it needs to be done carefully (more changes may be needed
than just the assertion removal) and doing so is outside this patch
scope, of course.

Thank you,

Alex.
Received on Thu Jul 18 2013 - 04:27:09 MDT