On 21/08/2016 9:08 a.m., Eduard Bagdasaryan wrote: > Hello, > > This patch fixes bug 4471. > > The bug was caused by the fact that Squid used only Last-Modified header > value for evaluating entry's last modification time while making an > internal revalidation request. So, without Last-Modified it was not > possible to correctly fill If-Modified-Since header value. As a result, > the revalidation request was not initiated when it should be. > > To fix this problem, we should generate If-Modified-Since based on other > data, showing how "old" the cached response is, such as Date and Age > header values. Both Date and Age header values are utilized while > calculating entry's timestamp. Therefore, we could use the timestamp if > Last-Modified is unavailable. > > XXX: HttpRequest::imslen support looks broken/deprecated. Using this > field inside StoreEntry::modifiedSince() leads to several useless checks > and probably may cause other [hidden] problems. >
Thanks for this. In src/Store.h: * please dont move the StoreEntry 'lastmod' member variable which exists between the "ON-DISK STORE_META_STD TLV field" marker comments. - To change anything between those markers we have to do a full cache versioning and up/down-grade compatibility dance. I think its best to keep all that trouble separate from this patch. - for now just document it with a comment saying how it should to be set instead of by assignment. in src/store.cc: * lastModifiedDelta() returning -1 when the actual delta is any -N value is wrong conceptually. - A delta should be the actual difference value -N < 0 < +N. - I've not had time right now to check how fixing that affects the caller logic though. That check will need doing before any commit so we can either produce proper delta's or comment why they get truncated to -1 in half the cases. in src/client_side_reply.cc: * please take the opportunity to cleanup any touched debugs() messages. Like this one: debugs(88, 5, "clientReplyContext::processExpired : lastmod " << entry->lastModified() ); should be: debugs(88, 5, "lastmod " << entry->lastModified()); - note function name and whitespace before ending bracket are gone. * for the hunk @@ -587 where it says "cannot revalidate entries without timestamp or Last-Modified header" - please add a note "XXX BUG 1890 objects without Date do not get one added". I think when that compliance bug is fixed an effective-LM will/should always be found. Cheers Amos _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev