[squid-dev] [PATCH] Revalidate without Last-Modified

[Amos Jeffries]

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