Re: Question regarding ESI Implementation

2011-09-29 Thread Jan Algermissen

On Sep 29, 2011, at 9:42 AM, Robert Collins wrote:

> On Thu, Sep 29, 2011 at 10:37 AM, Jan Algermissen
>  wrote:
>> Hi,
>> 
>> I am thinking about trying out some ideas building upon ESI 1.0 and would 
>> like to extend the ESI implementation of Squid. For personal use right now, 
>> but if turns out to be valuable I am happy to share it.
>> 
>> I downloaded the source yesterday and took a short look at the ESI parts.
>> 
>> Is there any form of documentation about the rationale behind the code 
>> snippets and the supported parts of ESI 1.0? What is the best way to get up 
>> to speed?
> 
> Theres no specific meta documentation that I recall. It should be
> pretty straight forward (but remember that squid is a single threaded
> non-blocking program - so its got to work with that style of
> programming).

Ah, I did not yet know Squid had an async programming model. Even better. The 
current ESI Implementation is non-blocking, too?


> 
>> Can you remember when the development roughly took place to help me digging 
>> through the developer archives?
> 
> early 2000's, uhm, I think 2003.

Thanks a lot.

Jan


> 
> -Rob



Re: [PATCH] SMP Caching: Core, IPC, Shared memory cache, and Rock Store

2011-09-29 Thread Alex Rousskov
On 09/07/2011 08:50 AM, Alex Rousskov wrote:
> On 09/07/2011 05:48 AM, Amos Jeffries wrote:

>> > configure.ac:
>> >  * rock store build conditions seem a bit out of sync with the intro
>> > documentation. The docs imply that rock will work with just blocking I/O
>> > without SMP mode.

> Yes, you still need to have SMP _support_, even if SMP mode is off. I
> think we can relax or even remove that requirement.

>> > The build checks requires that IpcIo and shm_open are supported simply
>> > to build it in. Could that be made to allow blocking-only builds with a
>> > configure-time check for IpcIo?

>> > Don't let this block the merge. Just make it clear in the docs and add
>> > to the TODO list if it is.

> Sounds good, we will work on it.

Hi Amos,

The series of 9 trunk patches posted by Dmitry yesterday (Subject:
"Make Rock work w/o SMP") make Rock Store build and work using Blocking
disk I/O when necessary. Even shared memory support is no longer
required to use Rock store in a non-SMP mode. Bugs notwithstanding, they
should address your concerns raised above. Please yell if we missing
something.

The same patches may fix Squid bug 3361 (at least).


HTH,

Alex.


Re: [MERGE] c++-refactor HttpHdrCc

2011-09-29 Thread Kinkie
On Thu, Sep 29, 2011 at 5:55 PM, Alex Rousskov
 wrote:
> On 09/29/2011 09:10 AM, Kinkie wrote:
>
>> How about proceeding like this?
>> This has already grown way past a "playground" or "refactoring" job
>> (it changes some behaviour).
>> We merge as-is, and then change the behaviour in a follow-up commit to trunk.
>
> Sounds good to me.
>
>
 Also, wouldn't that make the parser unnecessarily strict?
>>>
>>> Not sure what you mean. We got a malformed max-stale value. We have only
>>> two options when it comes to that value interpretation, I think:
>>>
>>>    A1. Treat it as valueless max-stale.
>>>    A2. Ignore it completely.
>>>
>>> Since valueless max-stale results in possibly very stale content being
>>> served to an unsuspecting client, I think #2 is much safer (and
>>> compliant with HTTP).
>>
>> You know way more than I do about this, so ok.
>>
>>> BTW, we have three options when it comes to forwarding the malformed
>>> directive:
>>>
>>>    B1. Forward our own valid directive.
>>>    B2. Forward nothing.
>>>    B3. Forward the malformed directive.
>>>
>>> While option B3 could be the best, it requires more parsing changes.
>>> Option B2 is much better than B1, IMO, and requires minimal changes.
>>>
>>> We currently implement A1 and B1. It is OK to treat this problem as
>>> outside your patch scope. I just hope the above summary will help
>>> somebody fix this later.
>>
>> Implementing B3 would be trivial, if we are allowed to just forward Cc
>> directives as we received them (I didn't dare do it due to my lack of
>> in-depth HTTP compliance knowledge).
>>
>> I will gladly do it as a followup patch as the other fix above.
>> Do you agree?
>
> Sure, but I doubt correct B3 implementation would be trivial.
>
> It would be indeed relatively easy to forward a malformed max-stale
> directive "as is" using the "other" member, but if Squid then sets its
> own max-stale, then we would be forwarding two max-stales, which is not
> kosher. Similarly, if we gain some code to filter directives (e.g.,
> using squid.conf ACLs), this simple B3 implementation would not filter
> malformed ones.
>
> It would be OK to ignore all these problems if we could somehow prevent
> new/future Squid code from bumping into them accidentally, but I cannot
> think of a neat way to do that.
>
> IMO, we should not attempt B3 and stick with B2 for now. B2 is trivial.

Ok.
If that's fine by you then I'll push ahead the merge of the
currently-implemented feature set to this evening, and start working
on A2 (B2 is a natural consequence of it).


-- 
    /kinkie


Re: [MERGE] c++-refactor HttpHdrCc

2011-09-29 Thread Alex Rousskov
On 09/29/2011 09:10 AM, Kinkie wrote:

> How about proceeding like this?
> This has already grown way past a "playground" or "refactoring" job
> (it changes some behaviour).
> We merge as-is, and then change the behaviour in a follow-up commit to trunk.

Sounds good to me.


>>> Also, wouldn't that make the parser unnecessarily strict?
>>
>> Not sure what you mean. We got a malformed max-stale value. We have only
>> two options when it comes to that value interpretation, I think:
>>
>>A1. Treat it as valueless max-stale.
>>A2. Ignore it completely.
>>
>> Since valueless max-stale results in possibly very stale content being
>> served to an unsuspecting client, I think #2 is much safer (and
>> compliant with HTTP).
> 
> You know way more than I do about this, so ok.
> 
>> BTW, we have three options when it comes to forwarding the malformed
>> directive:
>>
>>B1. Forward our own valid directive.
>>B2. Forward nothing.
>>B3. Forward the malformed directive.
>>
>> While option B3 could be the best, it requires more parsing changes.
>> Option B2 is much better than B1, IMO, and requires minimal changes.
>>
>> We currently implement A1 and B1. It is OK to treat this problem as
>> outside your patch scope. I just hope the above summary will help
>> somebody fix this later.
> 
> Implementing B3 would be trivial, if we are allowed to just forward Cc
> directives as we received them (I didn't dare do it due to my lack of
> in-depth HTTP compliance knowledge).
> 
> I will gladly do it as a followup patch as the other fix above.
> Do you agree?

Sure, but I doubt correct B3 implementation would be trivial.

It would be indeed relatively easy to forward a malformed max-stale
directive "as is" using the "other" member, but if Squid then sets its
own max-stale, then we would be forwarding two max-stales, which is not
kosher. Similarly, if we gain some code to filter directives (e.g.,
using squid.conf ACLs), this simple B3 implementation would not filter
malformed ones.

It would be OK to ignore all these problems if we could somehow prevent
new/future Squid code from bumping into them accidentally, but I cannot
think of a neat way to do that.

IMO, we should not attempt B3 and stick with B2 for now. B2 is trivial.


Thank you,

Alex.


Re: [MERGE] c++-refactor HttpHdrCc

2011-09-29 Thread Kinkie
On Thu, Sep 29, 2011 at 4:28 PM, Alex Rousskov
 wrote:
> On 09/29/2011 12:53 AM, Kinkie wrote:
>
          case CC_MAX_STALE:
 >> -
 >> -            if (!p || !httpHeaderParseInt(p, &cc->max_stale)) {
 >> +            if (!p || !httpHeaderParseInt(p, &max_stale) || max_stale 
 >> < 0) {
 >>                  debugs(65, 2, "cc: max-stale directive is valid 
 >> without value");
 >> -                cc->max_stale = -1;
 >> +                maxStale(MAX_STALE_ANY);
>
>
>>> > The above will treat max-stale=invalid as a valid valueless max-stale. I
>>> > think it would be better to treat malformed max-stale as no max-stale at
>>> > all. How about setting MAX_STALE_ANY _only_ when p is nil?
>
>
>> Fine by me, but from what I can tell I'm preserving the current behaviour.
>
>
> Yes you are preserving the current [incorrect] behavior except, perhaps,
> for negative values.

How about proceeding like this?
This has already grown way past a "playground" or "refactoring" job
(it changes some behaviour).
We merge as-is, and then change the behaviour in a follow-up commit to trunk.

>> Also, wouldn't that make the parser unnecessarily strict?
>
> Not sure what you mean. We got a malformed max-stale value. We have only
> two options when it comes to that value interpretation, I think:
>
>    A1. Treat it as valueless max-stale.
>    A2. Ignore it completely.
>
> Since valueless max-stale results in possibly very stale content being
> served to an unsuspecting client, I think #2 is much safer (and
> compliant with HTTP).

You know way more than I do about this, so ok.

> BTW, we have three options when it comes to forwarding the malformed
> directive:
>
>    B1. Forward our own valid directive.
>    B2. Forward nothing.
>    B3. Forward the malformed directive.
>
> While option B3 could be the best, it requires more parsing changes.
> Option B2 is much better than B1, IMO, and requires minimal changes.
>
> We currently implement A1 and B1. It is OK to treat this problem as
> outside your patch scope. I just hope the above summary will help
> somebody fix this later.

Implementing B3 would be trivial, if we are allowed to just forward Cc
directives as we received them (I didn't dare do it due to my lack of
in-depth HTTP compliance knowledge).

I will gladly do it as a followup patch as the other fix above.
Do you agree?

Thanks
-- 
    /kinkie


Re: [MERGE] c++-refactor HttpHdrCc

2011-09-29 Thread Alex Rousskov
On 09/29/2011 12:53 AM, Kinkie wrote:

>>>  case CC_MAX_STALE:
>>> >> -
>>> >> -if (!p || !httpHeaderParseInt(p, &cc->max_stale)) {
>>> >> +if (!p || !httpHeaderParseInt(p, &max_stale) || max_stale < 
>>> >> 0) {
>>> >>  debugs(65, 2, "cc: max-stale directive is valid without 
>>> >> value");
>>> >> -cc->max_stale = -1;
>>> >> +maxStale(MAX_STALE_ANY);


>> > The above will treat max-stale=invalid as a valid valueless max-stale. I
>> > think it would be better to treat malformed max-stale as no max-stale at
>> > all. How about setting MAX_STALE_ANY _only_ when p is nil?


> Fine by me, but from what I can tell I'm preserving the current behaviour.


Yes you are preserving the current [incorrect] behavior except, perhaps,
for negative values.


> Also, wouldn't that make the parser unnecessarily strict?

Not sure what you mean. We got a malformed max-stale value. We have only
two options when it comes to that value interpretation, I think:

A1. Treat it as valueless max-stale.
A2. Ignore it completely.

Since valueless max-stale results in possibly very stale content being
served to an unsuspecting client, I think #2 is much safer (and
compliant with HTTP).

BTW, we have three options when it comes to forwarding the malformed
directive:

B1. Forward our own valid directive.
B2. Forward nothing.
B3. Forward the malformed directive.

While option B3 could be the best, it requires more parsing changes.
Option B2 is much better than B1, IMO, and requires minimal changes.

We currently implement A1 and B1. It is OK to treat this problem as
outside your patch scope. I just hope the above summary will help
somebody fix this later.


Thank you,

Alex.


Re: Adding a reset option to the session helper

2011-09-29 Thread Henrik Nordström
tis 2011-09-27 klockan 07:32 +0100 skrev Andrew Beverley:

> I'd like to find a way around this. The best way that I can think of is
> to add an option to the session helper, to specify a URL that must be
> present for the timeout to reset. This URL would then be contained in
> the "continue" button on the splash page.

See the active mode which is intended for this kind of explicit session
start.

The URL match is done using Squid acls.

Session timeouts still applies.

Regards
Henrik



Re: Question regarding ESI Implementation

2011-09-29 Thread Robert Collins
On Thu, Sep 29, 2011 at 10:37 AM, Jan Algermissen
 wrote:
> Hi,
>
> I am thinking about trying out some ideas building upon ESI 1.0 and would 
> like to extend the ESI implementation of Squid. For personal use right now, 
> but if turns out to be valuable I am happy to share it.
>
> I downloaded the source yesterday and took a short look at the ESI parts.
>
> Is there any form of documentation about the rationale behind the code 
> snippets and the supported parts of ESI 1.0? What is the best way to get up 
> to speed?

Theres no specific meta documentation that I recall. It should be
pretty straight forward (but remember that squid is a single threaded
non-blocking program - so its got to work with that style of
programming).

> Can you remember when the development roughly took place to help me digging 
> through the developer archives?

early 2000's, uhm, I think 2003.

-Rob