[patch] refresh bugs

Hi

I've written a patch that is supposed to fix several problems with
how squid3 refreshes stale entries.

http://www.squid-cache.org/bugs/show_bug.cgi?id=1548

The following bugs are targeted:
Bug 1548 - Refresh where origin replies 304: cache is not updated,
future requests will still hit origin
Bug 1561 - Refresh where origin replies 200: two requests are made to
the origin instead of one

I've tested the fix by hand, feeding different requests in and
observing behaviour and logs. It does appear to fix the above bugs.
However, since it completely removes 3 functions, re-implements their
logic and does a few things differently, it could definitely do with
reviewing :)

General stuff:
1. A 304 from the origin server during a refresh now always updates
the existing entry's caching headers. It doesn't get its validators
checked, we just believe the origin. (See bug 1548 for discussion)
2. I have rewritten all the logic in three functions, and put them in
a single block at the end of handleIMSReply().
3. The three removed functions are:
        clientGetsOldEntry()
        handleIMSGiveClientUpdatedOldEntry()
        handleIMSGiveClientNewEntry()

Specific things I'd like comment on:
1. handleIMSGiveClientUpdatedOldEntry() used to HTTPMSGLOCK and
HTTPMSGUNLOCK the request. Is it okay to remove this? (I'm awaiting
some feedback from Duane on this one)
2. handleIMSGiveClientUpdatedOldEntry() used to set old_entry-
>timestamp to squid_curtime. Was this in fact correct?
3. handleIMSGiveClientNewEntry() used to call processMiss() - this
caused the duplicate request in bug 1561, and has gone. Was there a
reason for doing it this way that still stands?

I would be really grateful for any feedback/testing you can give this.

As an incentive, grep -c TCP_REFRESH your access logs... this patch
should blitz most of them :)

Cheers
Doug
Received on Mon May 01 2006 - 06:28:47 MDT