Re: your suggestion for range_offset_limit

Matthew Morgan wrote:
> Sorry it's taking me so long to get this done, but I do have a question.
>
> You suggested making getRangeOffsetLimit a member of HttpReply. There
> are two places where this method currently needs to be called: one is
> CheckQuickAbort2() in store_client.cc. This one will be easy, as I can
> just do entry->getReply()->getRangeOffsetLimit().
>
> The other is HttpStateData::decideIfWeDoRanges in http.cc. Here, all we
> have access to is an HttpRequest object. I looked through the source to
> see if I could find where a request owned or had access to a reply, but
> I don't see anything like that. If getRangeOffsetLimit were a member of
> HttpReply, what do you suggest doing here? I could make a static
> version of the method, but that wouldn't allow caching the result.

Ah. I see. Quite right.

After a bit more though I find my original request a bit weird.

Yes it should be a _Request_ member and do its caching there. You can go
ahead with that now while we discuss whether to do a slight tweak on top
of the basic feature.

[cc'ing squid-dev so others can provide input]

I'm not certain of the behavior we want here if we do open the ACLs to
reply details. Some discussion is in order.

Simple way would be to not cache the lookup the first time when reply
details are not provided.

It would mean making it return potentially two different values across
the transaction.

  1) based on only request detail to
  and other on request+reply details. decide if a range request to possible.
and then
2) based on additional reply details to see if the abort could be done.

No problem if the reply details cause an increase in the limit. But if
they restrict it we enter grounds of potentially making a request then
canceling it and being unable to store the results.

Or, taking the maximum of the two across two calls? so it can only increase.
  would be slightly trickier involving a flag a well to short-circuit
the reply lookups instead of just a magic cache value.

Am I seriously over-thinking things today?

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE7 or 3.0.STABLE20
   Current Beta Squid 3.1.0.15