Re: [PATCH] reply_from_cache and reply_to_cache

On 12/18/2013 04:16 AM, Amos Jeffries wrote:
> On 16/10/2013 4:36 a.m., Alex Rousskov wrote:
>>
>> Going forward, I think we need to decide:
>>
>> A) Whether altering the existing "cache" directive semantics is
>> desirable. If it is a good idea, we can remove or deprecate that option
>> and ignore its [end-of-life] existence when deciding how to structure
>> the new directives.
>>
>> B) Whether we should keep decision point 1 (action before hit/miss
>> determination and partial loading of the response is made).
>>
>>
>> I am not sure about (A) but I suspect we may want keep the "fast"
>> decision point in (B) if for no other reason than performance of simple
>> cases.

> I can agree with (B) and putting off the review of the "cache" directive
> for some future date if necessary.

If we keep "cache", then "lookup_hit" may be implemented to support fast
ACLs only. If we do not keep "cache", then "lookup_hit" should probably
be implemented to support slow ACLs (and reuse most of the existing
"cache" code for that). That is why it would be nice to make the
decision regarding "cache" before implementing "lookup_hit".

> I've had a read through the version of your patch cleaned up and
> submitted as fix for bug 3937. It appears to implement what you are
> talking about as (B). Are there any additional changes to be made on that?

After updating my changes, I looked through the v3.4 patch posted to
bugzilla and noticed that it uses stale Config.accessList member names.
Committed changes also better document the lack of slow ACL support by
the new directives. I recommend that you use the committed changes
(trunk r13190) for the official v3.4 port, but it is your call.

> I am okay with a +1 on it and those updated names.

AFAICT, that is what trunk r13190 contains, essentially.

Cheers,

Alex.
Received on Wed Dec 18 2013 - 17:35:34 MST