Re: CARP improvements

Hi,

On Sun, Jul 31, 2011 at 4:24 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> On 31/07/11 07:26, Kinkie wrote:
>>
>> Hi all,
>>   attached is a first revision of the patch for review. I've build-
>> and run- tested it.
>> Only issue not fully investigated is that when an user login is
>> specified in the request, carpSelectParent is not invoked at all. This
>> is however external from the carp code itself; only effect it has is
>> that the "login" carp key selector is actually useless.
>>
>
> When login is in headers rather than URL you may need to use:

Hm.. thorny issue here, as there are two use cases: forward and reverse proxy.
In both cases, the only identity it makes sense to use for a routing
decision is the user's identity on the origin server, not on this
cache.
With your code suggestion, what will be used is the user's identity on
"this" cache in both cases, which is IMVHO wrong.
Maybe we need to go lower-level here and fetch the headers' contents directly?

>
>  if (tp->options.carp_key.login) {
>      if (request->login && *request->login) {
>          key.append(request->login);
>      }
> #if USE_AUTH
>        else if (request->auth_user_request) {
>          key.append(request->auth_user_request->username());
>      }
> #endif
>          key.append('@');
>  }
>
> I'm not sure if you want auth auth_user_request->type() and public tokens
> embeded in the header case as well.
>
>
>> On Mon, Jul 18, 2011 at 2:01 PM, Amos Jeffries<squid3_at_treenet.co.nz>
>>  wrote:
>>>
>>> On 18/07/11 22:27, Kinkie wrote:
>>>>
>>>> Hi guys,
>>>>   I would like some input on how to best tweak CARP with a very small
>>>> change:
>>>> as it is now the canonical URL is used to define the hash for next
>>>> parent.
>>>> It is very efficient - even too much.
>>>>
>>>> I'd like to add a variant to it so that only the host part of the URL
>>>> is used as initial key for the hashing.
>>>> This will decrease the effectiveness of the balancing, but make it
>>>> easier to cope with some websites which do not like it when requests
>>>> travel across different routes.
>>>>
>>>> There are three ways I can think of to address this:
>>>> - change the way CARP works - it's an one-liner
>>>> - new global setting
>>>> - new carp option (e.g. hcarp or host-carp)
>>>>
>>>> What do you think is the best way to implement this?
>>>
>>> I think a slightly more flexible way would be a carp-key=XX parameter.
>>> Where
>>> one can potentially set (scheme,host,port,path,params) tags. That would
>>> allow a bit more tuning of the mask.
>
>
> src/cache_cf.cc:
>  * Can you use strncasecmp instead of casecmp?
>   so the parser can work on const char* and avoid strdup() and \0 writes

Ok.

> src/carp.cc:
>  * s/"cornercases"/"corner cases"
>  * also a bit wrong, this code does not cover _all_ corner cases as
> canonical. Things like unknown method and unknown protocols will use the
> else condition. Which in fact defaults to the custom key.

Ok, then it is wrong; it should default on canonical: it's the safe fallback.
One thing we need not concern with is interopearbility. We're rolling
our own, a mixed cache environment is not safe except when using plain
CARP (btw: is there anyone else implementing CARP nowadays? Or do
other caches use custom balancing algorithms as we're designing now?)

>  At least the docs needs to say "corner cases should use canonical URL"

>  Can you explain why that is even needed though?  Both CONNECT and URN cases
> have all keys with known values. Albeit some values are "".

Probably ignorance on my side, and preferring to err on the safe side.
I can change that if you think it'd be wiser.

-- 
    /kinkie