Hi Lukas,

On Sun, Dec 30, 2018 at 09:52:33PM +0100, Lukas Tribus wrote:
> In my tests your patch does the job and redispatch does work for
> consistent source or uri hashing, but only if hash-balance-factor is
> set. If I comment hash-balance-factor out, the redispatch does not
> occur:
> 
> #balance uri
> balance source
> hash-type consistent
> hash-balance-factor 150
> server primary-fail 10.0.0.199:80 maxconn 200 weight 256 # unreachable
> server backup-ok 10.0.0.254:80 maxconn 200 weight 1
> 
> 
> Not sure why this happens.

Bah, of course, guess who is as stupid at the end of the year as at the
beginning ? Me of course!

The issue is here :
      while (p->lbprm.chash.balance_factor && (nsrv == avoid || 
!chash_server_is_eligible(nsrv))) {

It should be this :
      while (nsrv == avoid || (p->lbprm.chash.balance_factor && 
!chash_server_is_eligible(nsrv))) {

I'm attaching an updated patch. If it's OK, feel free to adjust it
according to your taste and to put your initial commit message on
it and your previous doc updates. I think we can safely backport it
this way.

Thanks!
Willy
diff --git a/include/proto/lb_chash.h b/include/proto/lb_chash.h
index a0ebf696e..679dff363 100644
--- a/include/proto/lb_chash.h
+++ b/include/proto/lb_chash.h
@@ -28,7 +28,7 @@
 
 void chash_init_server_tree(struct proxy *p);
 struct server *chash_get_next_server(struct proxy *p, struct server 
*srvtoavoid);
-struct server *chash_get_server_hash(struct proxy *p, unsigned int hash);
+struct server *chash_get_server_hash(struct proxy *p, unsigned int hash, const 
struct server *avoid);
 
 #endif /* _PROTO_LB_CHASH_H */
 
diff --git a/src/backend.c b/src/backend.c
index 3c1620bbb..c92e76172 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -165,7 +165,7 @@ void update_backend_weight(struct proxy *px)
  * If any server is found, it will be returned. If no valid server is found,
  * NULL is returned.
  */
-static struct server *get_server_sh(struct proxy *px, const char *addr, int 
len)
+static struct server *get_server_sh(struct proxy *px, const char *addr, int 
len, const struct server *avoid)
 {
        unsigned int h, l;
 
@@ -186,7 +186,7 @@ static struct server *get_server_sh(struct proxy *px, const 
char *addr, int len)
                h = full_hash(h);
  hash_done:
        if (px->lbprm.algo & BE_LB_LKUP_CHTREE)
-               return chash_get_server_hash(px, h);
+               return chash_get_server_hash(px, h, avoid);
        else
                return map_get_server_hash(px, h);
 }
@@ -203,7 +203,7 @@ static struct server *get_server_sh(struct proxy *px, const 
char *addr, int len)
  * algorithm out of a tens because it gave him the best results.
  *
  */
-static struct server *get_server_uh(struct proxy *px, char *uri, int uri_len)
+static struct server *get_server_uh(struct proxy *px, char *uri, int uri_len, 
const struct server *avoid)
 {
        unsigned int hash = 0;
        int c;
@@ -239,7 +239,7 @@ static struct server *get_server_uh(struct proxy *px, char 
*uri, int uri_len)
                hash = full_hash(hash);
  hash_done:
        if (px->lbprm.algo & BE_LB_LKUP_CHTREE)
-               return chash_get_server_hash(px, hash);
+               return chash_get_server_hash(px, hash, avoid);
        else
                return map_get_server_hash(px, hash);
 }
@@ -253,7 +253,7 @@ static struct server *get_server_uh(struct proxy *px, char 
*uri, int uri_len)
  * is returned. If any server is found, it will be returned. If no valid server
  * is found, NULL is returned.
  */
-static struct server *get_server_ph(struct proxy *px, const char *uri, int 
uri_len)
+static struct server *get_server_ph(struct proxy *px, const char *uri, int 
uri_len, const struct server *avoid)
 {
        unsigned int hash = 0;
        const char *start, *end;
@@ -296,7 +296,7 @@ static struct server *get_server_ph(struct proxy *px, const 
char *uri, int uri_l
                                        hash = full_hash(hash);
 
                                if (px->lbprm.algo & BE_LB_LKUP_CHTREE)
-                                       return chash_get_server_hash(px, hash);
+                                       return chash_get_server_hash(px, hash, 
avoid);
                                else
                                        return map_get_server_hash(px, hash);
                        }
@@ -315,7 +315,7 @@ static struct server *get_server_ph(struct proxy *px, const 
char *uri, int uri_l
 /*
  * this does the same as the previous server_ph, but check the body contents
  */
-static struct server *get_server_ph_post(struct stream *s)
+static struct server *get_server_ph_post(struct stream *s, const struct server 
*avoid)
 {
        unsigned int hash = 0;
        struct http_txn *txn  = s->txn;
@@ -370,7 +370,7 @@ static struct server *get_server_ph_post(struct stream *s)
                                        hash = full_hash(hash);
 
                                if (px->lbprm.algo & BE_LB_LKUP_CHTREE)
-                                       return chash_get_server_hash(px, hash);
+                                       return chash_get_server_hash(px, hash, 
avoid);
                                else
                                        return map_get_server_hash(px, hash);
                        }
@@ -396,7 +396,7 @@ static struct server *get_server_ph_post(struct stream *s)
  * is returned. If any server is found, it will be returned. If no valid server
  * is found, NULL is returned.
  */
-static struct server *get_server_hh(struct stream *s)
+static struct server *get_server_hh(struct stream *s, const struct server 
*avoid)
 {
        unsigned int hash = 0;
        struct http_txn *txn  = s->txn;
@@ -466,13 +466,13 @@ static struct server *get_server_hh(struct stream *s)
                hash = full_hash(hash);
  hash_done:
        if (px->lbprm.algo & BE_LB_LKUP_CHTREE)
-               return chash_get_server_hash(px, hash);
+               return chash_get_server_hash(px, hash, avoid);
        else
                return map_get_server_hash(px, hash);
 }
 
 /* RDP Cookie HASH.  */
-static struct server *get_server_rch(struct stream *s)
+static struct server *get_server_rch(struct stream *s, const struct server 
*avoid)
 {
        unsigned int hash = 0;
        struct proxy    *px   = s->be;
@@ -511,13 +511,13 @@ static struct server *get_server_rch(struct stream *s)
                hash = full_hash(hash);
  hash_done:
        if (px->lbprm.algo & BE_LB_LKUP_CHTREE)
-               return chash_get_server_hash(px, hash);
+               return chash_get_server_hash(px, hash, avoid);
        else
                return map_get_server_hash(px, hash);
 }
 
 /* random value  */
-static struct server *get_server_rnd(struct stream *s)
+static struct server *get_server_rnd(struct stream *s, const struct server 
*avoid)
 {
        unsigned int hash = 0;
        struct proxy  *px = s->be;
@@ -528,7 +528,7 @@ static struct server *get_server_rnd(struct stream *s)
 
        /* ensure all 32 bits are covered as long as RAND_MAX >= 65535 */
        hash = ((uint64_t)random() * ((uint64_t)RAND_MAX + 1)) ^ random();
-       return chash_get_server_hash(px, hash);
+       return chash_get_server_hash(px, hash, avoid);
 }
 
 /*
@@ -639,7 +639,7 @@ int assign_server(struct stream *s)
                case BE_LB_LKUP_MAP:
                        if ((s->be->lbprm.algo & BE_LB_KIND) == BE_LB_KIND_RR) {
                                if ((s->be->lbprm.algo & BE_LB_PARM) == 
BE_LB_RR_RANDOM)
-                                       srv = get_server_rnd(s);
+                                       srv = get_server_rnd(s, prev_srv);
                                else if (s->be->lbprm.algo & BE_LB_LKUP_CHTREE)
                                        srv = chash_get_next_server(s->be, 
prev_srv);
                                else
@@ -658,12 +658,12 @@ int assign_server(struct stream *s)
                                if (conn && conn->addr.from.ss_family == 
AF_INET) {
                                        srv = get_server_sh(s->be,
                                                            (void *)&((struct 
sockaddr_in *)&conn->addr.from)->sin_addr,
-                                                           4);
+                                                           4, prev_srv);
                                }
                                else if (conn && conn->addr.from.ss_family == 
AF_INET6) {
                                        srv = get_server_sh(s->be,
                                                            (void *)&((struct 
sockaddr_in6 *)&conn->addr.from)->sin6_addr,
-                                                           16);
+                                                           16, prev_srv);
                                }
                                else {
                                        /* unknown IP family */
@@ -678,7 +678,7 @@ int assign_server(struct stream *s)
                                        break;
                                srv = get_server_uh(s->be,
                                                    c_ptr(&s->req, 
-http_uri_rewind(&s->txn->req)),
-                                                   s->txn->req.sl.rq.u_l);
+                                                   s->txn->req.sl.rq.u_l, 
prev_srv);
                                break;
 
                        case BE_LB_HASH_PRM:
@@ -688,22 +688,22 @@ int assign_server(struct stream *s)
 
                                srv = get_server_ph(s->be,
                                                    c_ptr(&s->req, 
-http_uri_rewind(&s->txn->req)),
-                                                   s->txn->req.sl.rq.u_l);
+                                                   s->txn->req.sl.rq.u_l, 
prev_srv);
 
                                if (!srv && s->txn->meth == HTTP_METH_POST)
-                                       srv = get_server_ph_post(s);
+                                       srv = get_server_ph_post(s, prev_srv);
                                break;
 
                        case BE_LB_HASH_HDR:
                                /* Header Parameter hashing */
                                if (!s->txn || s->txn->req.msg_state < 
HTTP_MSG_BODY)
                                        break;
-                               srv = get_server_hh(s);
+                               srv = get_server_hh(s, prev_srv);
                                break;
 
                        case BE_LB_HASH_RDP:
                                /* RDP Cookie hashing */
-                               srv = get_server_rch(s);
+                               srv = get_server_rch(s, prev_srv);
                                break;
 
                        default:
diff --git a/src/lb_chash.c b/src/lb_chash.c
index 7e188ac41..186e87a5f 100644
--- a/src/lb_chash.c
+++ b/src/lb_chash.c
@@ -313,9 +313,10 @@ int chash_server_is_eligible(struct server *s)
  * the closest distance from the value of <hash>. Doing so ensures that even
  * with a well imbalanced hash, if some servers are close to each other, they
  * will still both receive traffic. If any server is found, it will be 
returned.
+ * It will also skip server <avoid> if the hash result ends on this one.
  * If no valid server is found, NULL is returned.
  */
-struct server *chash_get_server_hash(struct proxy *p, unsigned int hash)
+struct server *chash_get_server_hash(struct proxy *p, unsigned int hash, const 
struct server *avoid)
 {
        struct eb32_node *next, *prev;
        struct server *nsrv, *psrv;
@@ -367,7 +368,7 @@ struct server *chash_get_server_hash(struct proxy *p, 
unsigned int hash)
        }
 
        loop = 0;
-       while (p->lbprm.chash.balance_factor && 
!chash_server_is_eligible(nsrv)) {
+       while (nsrv == avoid || (p->lbprm.chash.balance_factor && 
!chash_server_is_eligible(nsrv))) {
                next = eb32_next(next);
                if (!next) {
                        next = eb32_first(root);

Reply via email to