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

Reply via email to