On 27 Apr 2006, at 11:10, Duane Wessels wrote:
On Tue, 25 Apr 2006, Doug Dixon wrote:
It looks like the call to refreshCheck() is only used to short-
circuit the function (if you can call it a short circuit, given
the size of the callout) and to update some statistics.
The main, and original, purpose of refreshCheck() is to check the
request
against the 'refresh_pattern' configuration values from squid.conf.
Yep, refreshCheck() is used elsewhere and that's fine - it's just the
call from refreshIsCachable() that I'm questioning.
I can't see the reason stats being used anywhere, but maybe I
overlooked something. (And anyway... should this function even be
updating the refreshCounts[rcStore].total stat, given the fact
this just checks a couple of flags?)
The 'reason' return value and related statistcs are definately used.
You can see refresh statistics in the cache manager 'refresh' page.
Ah sorry, you're right :)
However, the return value can be calculated without calling it,
and the statistic it updates is never used. Would something like
this be cleaner/quicker and still correct?
I'm not sure which function "it" refers to.
We can't NOT call refreshCheck(), ala your suggested
refreshIsCachable()
replacement. refreshIsCachable() could be made slightly shorter and
cleaner, but I don't think its worth the trouble.
The return value of refreshIsCachable() can be calculated without
making a call to refreshCheck().
I.e. you can remove the call to refreshCheck() from refreshIsCachable
(), and refreshIsCachable() will still return the correct result.
The only side-effect is the loss of the refreshCounts[rcStore]
stats... but since the meaning of this stat is (per CacheMgr cgi)
"calls to refresh check", this stat should always be zero if we don't
make the call. I.e. if we lose the call to refreshCheck, the right
thing to do is lose the statistic.
Cheers
Doug
