>> 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:

As discussed, I'm dropping login as a possible key.

> 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

Done. Other options would benefit of the same treatment, but it's
outside the scope of this effort.

> src/carp.cc:
>  * s/"cornercases"/"corner cases"

ok

>  * 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.
>  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 "".

Done differently. New code blindly tries to use the custom key. If at
the end of the processing it is empty (maybe because the user is
asking to balance on path for a CONNECT request), then urlCanonical is
used. This should automatically cover all corner cases.

New revision is attached.
It also adds support for no-copy config-file parsing, as discussed on IRC.

-- 
    /kinkie
=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc	2011-07-26 23:09:58 +0000
+++ src/cache_cf.cc	2011-08-01 16:38:20 +0000
@@ -2244,6 +2244,28 @@
                 fatalf("parse_peer: non-parent carp peer %s/%d\n", p->host, p->http_port);
 
             p->options.carp = 1;
+        } else if (!strncasecmp(token, "carp-key", 8)) {
+            if (p->options.carp != 1)
+                fatalf("parse_peer: carp-key specified on non-carp peer %s/%d\n", p->host, p->http_port);
+            p->options.carp_key.set=1;
+            char *nextkey=token+strlen("carp-key="), *key=nextkey;
+            for (; key; key = nextkey) {
+            	nextkey=strchr(key,',');
+            	if (nextkey) ++nextkey; // skip the comma, if a comma is found.ut that
+            	if (0==strncasecmp(key,"scheme",6)) {
+            		p->options.carp_key.scheme=1;
+            	} else if (0==strncasecmp(key,"host",4)) {
+            		p->options.carp_key.host=1;
+            	} else if (0==strncasecmp(key,"port",4)) {
+            		p->options.carp_key.port=1;
+            	} else if (0==strncasecmp(key,"path",4)) {
+            		p->options.carp_key.path=1;
+            	} else if (0==strncasecmp(key,"params",6)) {
+            		p->options.carp_key.params=1;
+            	} else {
+            		fatalf("invalid carp-key '%s'",key);
+            	}
+            }
         } else if (!strcasecmp(token, "userhash")) {
 #if USE_AUTH
             if (p->type != PEER_PARENT)

=== modified file 'src/carp.cc'
--- src/carp.cc	2010-10-28 18:52:59 +0000
+++ src/carp.cc	2011-08-01 14:38:36 +0000
@@ -35,8 +35,10 @@
  */
 
 #include "squid.h"
+#include "HttpRequest.h"
 #include "mgr/Registration.h"
 #include "Store.h"
+#include "URLScheme.h"
 
 #define ROTATE_LEFT(x, n) (((x) << (n)) | ((x) >> (32-(n))))
 
@@ -164,35 +166,69 @@
 carpSelectParent(HttpRequest * request)
 {
     int k;
-    const char *c;
     peer *p = NULL;
     peer *tp;
     unsigned int user_hash = 0;
     unsigned int combined_hash;
     double score;
     double high_score = 0;
-    const char *key = NULL;
 
     if (n_carp_peers == 0)
         return NULL;
 
-    key = urlCanonical(request);
-
     /* calculate hash key */
-    debugs(39, 2, "carpSelectParent: Calculating hash for " << key);
-
-    for (c = key; *c != 0; c++)
-        user_hash += ROTATE_LEFT(user_hash, 19) + *c;
+    debugs(39, 2, "carpSelectParent: Calculating hash for " << urlCanonical(request));
 
     /* select peer */
     for (k = 0; k < n_carp_peers; k++) {
+    	String key;
         tp = carp_peers[k];
+        if (tp->options.carp_key.set) {
+            //this code follows urlCanonical's pattern.
+            //   corner cases should use the canonical URL
+            if (tp->options.carp_key.scheme) {
+                // temporary, until bug 1961 URL handling is fixed.
+                const URLScheme sch = request->protocol;
+                key.append(sch.const_str());
+                if (key.size()) //if the scheme is not empty
+                    key.append("://");
+            }
+            if (tp->options.carp_key.host) {
+                key.append(request->GetHost());
+            }
+            if (tp->options.carp_key.port) {
+                static char portbuf[7];
+                snprintf(portbuf,7,":%d", request->port);
+                key.append(portbuf);
+            }
+            if (tp->options.carp_key.path) {
+                String::size_type pos;
+                if ((pos=request->urlpath.find('?'))!=String::npos)
+                    key.append(request->urlpath.substr(0,pos));
+                else
+                    key.append(request->urlpath);
+            }
+            if (tp->options.carp_key.params) {
+                String::size_type pos;
+                if ((pos=request->urlpath.find('?'))!=String::npos)
+                    key.append(request->urlpath.substr(pos,request->urlpath.size()));
+            }
+    	}
+        // if the url-based key is empty, e.g. because the user is
+        // asking to balance on the path but the request doesn't supply any,
+        // then fall back to canonical URL
+
+        if (key.size()==0)
+            key=urlCanonical(request);
+
+        for (const char *c = key.rawBuf(), *e=key.rawBuf()+key.size(); c < e; c++)
+            user_hash += ROTATE_LEFT(user_hash, 19) + *c;
         combined_hash = (user_hash ^ tp->carp.hash);
         combined_hash += combined_hash * 0x62531965;
         combined_hash = ROTATE_LEFT(combined_hash, 21);
         score = combined_hash * tp->carp.load_multiplier;
-        debugs(39, 3, "carpSelectParent: " << tp->name << " combined_hash " << combined_hash  <<
-               " score " << std::setprecision(0) << score);
+        debugs(39, 3, "carpSelectParent: key=" << key << " name=" << tp->name << " combined_hash=" << combined_hash  <<
+               " score=" << std::setprecision(0) << score);
 
         if ((score > high_score) && peerHTTPOkay(tp, request)) {
             p = tp;

=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2011-07-27 01:41:00 +0000
+++ src/cf.data.pre	2011-08-01 14:04:58 +0000
@@ -2168,6 +2168,14 @@
 			than the Squid default location.
 	
 	
+	==== CARP OPTIONS ====
+	
+	carp-key=key-specification
+			use a different key than the full URL to hash against the peer.
+			the key-specification is a comma-separated list of the keywords			
+			scheme, host, port, path, params
+			Order is not important.
+	
 	==== ACCELERATOR / REVERSE-PROXY OPTIONS ====
 	
 	originserver	Causes this parent to be contacted as an origin server.

=== modified file 'src/structs.h'
--- src/structs.h	2011-06-23 08:33:13 +0000
+++ src/structs.h	2011-08-01 14:01:22 +0000
@@ -855,6 +855,14 @@
 #endif
         unsigned int allow_miss:1;
         unsigned int carp:1;
+        struct {
+        	unsigned int set:1; //If false, whole url is to be used. Overrides others
+        	unsigned int scheme:1;
+        	unsigned int host:1;
+        	unsigned int port:1;
+        	unsigned int path:1;
+        	unsigned int params:1;
+        } carp_key;
 #if USE_AUTH
         unsigned int userhash:1;
 #endif

Reply via email to