[PATCH] : Prevent redispatcher from selecting the same server

From 7ece40b892a91f0d7fc673b6e5977766ab3e047b Mon Sep 17 00:00:00 2001 From: Krzysztof Piotr Oledzki <ole#ans.pl> Date: Sun, 3 Feb 2008 23:44:44 +0100
Subject: [MAJOR]: Prevent redispatcher from selecting the same server

When haproxy decides that session needs to be redispatched it chose a server, but there is no guarantee for it to be a different than the current one. So, it often happens that selected server is exactly the same that it was previously and client ends up with a 503 error anyway, especially when one sever has much bigger weight than others.

This patch fixes FWRR and RR algos. It also delays SN_DIRECT/SN_REDISP operations and http_flush_cookie_flags call so they are executed only when a connection was truly redispatched not only intended to.

---
 include/proto/backend.h |   16 ++++++++---
 src/backend.c           |   64 +++++++++++++++++++++++++++++++++-------------
 src/checks.c            |    6 +---
 src/proto_http.c        |    6 +---
 4 files changed, 61 insertions(+), 31 deletions(-)

diff --git a/include/proto/backend.h b/include/proto/backend.h
index 444e53a..bbd8917 100644
--- a/include/proto/backend.h
+++ b/include/proto/backend.h


@@ -50,10 +50,10 @@ void fwrr_init_server_groups(struct proxy *p);

  * If any server is found, it will be returned and px->lbprm.map.rr_idx will be updated
  * to point to the next server. If no valid server is found, NULL is returned.
  */
-static inline struct server *get_server_rr_with_conns(struct proxy *px)
+static inline struct server *get_server_rr_with_conns(struct proxy *px, struct server *srvtoavoid)
 {
 	int newidx;
-	struct server *srv;
+	struct server *srv, *avoided;
 
 	if (px->lbprm.tot_weight == 0)
 		return NULL;


@@ -65,17 +65,23 @@ static inline struct server *get_server_rr_with_conns(struct proxy *px)

 		px->lbprm.map.rr_idx = 0;
 	newidx = px->lbprm.map.rr_idx;
 
+	avoided = NULL;
 	do {
 		srv = px->lbprm.map.srv[newidx++];
 		if (!srv->maxconn || srv->cur_sess < srv_dynamic_maxconn(srv)) {
-			px->lbprm.map.rr_idx = newidx;
-			return srv;
+			/* make sure it is not the server we are trying to exclude... */
+			if (srv != srvtoavoid) {
+				px->lbprm.map.rr_idx = newidx;
+				return srv;
+			}
+			avoided = srv;	/* ...but remember that is was selected yet avoided */
 		}
 		if (newidx == px->lbprm.tot_weight)
 			newidx = 0;
 	} while (newidx != px->lbprm.map.rr_idx);
 
-	return NULL;
+	/* return NULL or srvtoavoid if found */
+	return avoided;
 }
 
 
diff --git a/src/backend.c b/src/backend.c
index 5ea6590..b545e91 100644
--- a/src/backend.c
+++ b/src/backend.c


@@ -727,11 +727,10 @@ static inline void fwrr_update_position(struct fwrr_group *grp, struct server *s

  * the init tree if appropriate. If both trees are empty, return NULL.
  * Saturated servers are skipped and requeued.
  */
-static struct server *fwrr_get_next_server(struct proxy *p)
+static struct server *fwrr_get_next_server(struct proxy *p, struct server *srvtoavoid)
 {
-	struct server *srv;
+	struct server *srv, *full, *avoided;
 	struct fwrr_group *grp;
-	struct server *full;
 	int switched;
 
 	if (p->srv_act)


@@ -744,6 +743,7 @@ static struct server *fwrr_get_next_server(struct proxy *p)

 		return NULL;
 
 	switched = 0;
+	avoided = 0;
 	full = NULL; /* NULL-terminated list of saturated servers */
 	while (1) {
 		/* if we see an empty group, let's first try to collect weights


@@ -759,8 +759,13 @@ static struct server *fwrr_get_next_server(struct proxy *p)

 			srv = fwrr_get_server_from_group(grp);
 			if (srv)
 				break;
-			if (switched)
+			if (switched) {
+				if (avoided) {
+					srv = avoided;
+					break;
+				}
 				goto requeue_servers;
+			}
 			switched = 1;
 			fwrr_switch_trees(grp);
 


@@ -774,10 +779,15 @@ static struct server *fwrr_get_next_server(struct proxy *p)

 		fwrr_update_position(grp, srv);
 		fwrr_dequeue_srv(srv);
 		grp->curr_pos++;
-		if (!srv->maxconn || srv->cur_sess < srv_dynamic_maxconn(srv))
-			break;
+		if (!srv->maxconn || srv->cur_sess < srv_dynamic_maxconn(srv)) {
+			/* make sure it is not the server we are trying to exclude... */
+			if (srv != srvtoavoid)
+				break;
+
+			avoided = srv; /* ...but remember that is was selected yet avoided */
+		}
 
-		/* the server is saturated, let's chain it for later reinsertion */
+		/* the server is saturated or avoided, let's chain it for later reinsertion */
 		srv->next_full = full;
 		full = srv;
 	}


@@ -786,6 +796,9 @@ static struct server *fwrr_get_next_server(struct proxy *p)

 	fwrr_queue_srv(srv);
 
  requeue_servers:
+	/* Requeue all extracted servers. If full==srv then it was
+	 * avoided (unsucessfully) and chained, omit it now.
+	 */
 	if (unlikely(full)) {
 		if (switched) {
 			/* the tree has switched, requeue all extracted servers


@@ -793,7 +806,8 @@ static struct server *fwrr_get_next_server(struct proxy *p)

 			 * their weight matters.
 			 */
 			do {
-				fwrr_queue_by_weight(grp->init, full);
+				if (likely(full != srv))
+					fwrr_queue_by_weight(grp->init, full);
 				full = full->next_full;
 			} while (full);
 		} else {


@@ -801,7 +815,8 @@ static struct server *fwrr_get_next_server(struct proxy *p)

 			 * so that they regain their expected place.
 			 */
 			do {
-				fwrr_queue_srv(full);
+				if (likely(full != srv))
+					fwrr_queue_srv(full);
 				full = full->next_full;
 			} while (full);
 		}


@@ -914,7 +929,7 @@ int assign_server(struct session *s)

 
 			switch (s->be->lbprm.algo & BE_LB_ALGO) {
 			case BE_LB_ALGO_RR:
-				s->srv = fwrr_get_next_server(s->be);
+				s->srv = fwrr_get_next_server(s->be, s->srv);
 				if (!s->srv)
 					return SRV_STATUS_FULL;
 				break;


@@ -943,7 +958,7 @@ int assign_server(struct session *s)

 						       s->txn.req.sl.rq.u_l);
 				if (!s->srv) {
 					/* parameter not found, fall back to round robin on the map */
-					s->srv = get_server_rr_with_conns(s->be);
+					s->srv = get_server_rr_with_conns(s->be, s->srv);
 					if (!s->srv)
 						return SRV_STATUS_FULL;
 				}


@@ -1053,6 +1068,7 @@ int assign_server_address(struct session *s)

 int assign_server_and_queue(struct session *s)
 {
 	struct pendconn *p;
+	struct server *srv;
 	int err;
 
 	if (s->pend_pos)


@@ -1060,9 +1076,8 @@ int assign_server_and_queue(struct session *s)

 
 	if (s->flags & SN_ASSIGNED) {
 		if (s->srv && s->srv->maxqueue > 0 && s->srv->nbpend >= s->srv->maxqueue) {
-			s->flags &= ~(SN_DIRECT | SN_ASSIGNED | SN_ADDR_SET);
-			s->srv = NULL;
-			http_flush_cookie_flags(&s->txn);
+			/* it's left to the dispatcher to choose a server */
+			s->flags &= ~(SN_ASSIGNED | SN_ADDR_SET);
 		} else {
 			/* a server does not need to be assigned, perhaps because we're in
 			 * direct mode, or in dispatch or transparent modes where the server


@@ -1081,7 +1096,22 @@ int assign_server_and_queue(struct session *s)

 	}
 
 	/* a server needs to be assigned */
+	srv = s->srv;
 	err = assign_server(s);
+
+	if (srv && srv != s->srv) {
+		/* This session was previously dispatched to another server:
+		 *  - clear SN_DIRECT
+		 *  - flush a cookie
+		 *  - set SN_REDISP if it was successfully redispatched
+		 */
+
+		s->flags &= ~(SN_DIRECT);
+		http_flush_cookie_flags(&s->txn);
+		if (srv)
+			s->flags |= SN_REDISP;
+	}
+
 	switch (err) {
 	case SRV_STATUS_OK:
 		/* in balance mode, we might have servers with connection limits */


@@ -1409,10 +1439,8 @@ int srv_retryable_connect(struct session *t)

 	}
 	t->be->redispatches++;
 
-	t->flags &= ~(SN_DIRECT | SN_ASSIGNED | SN_ADDR_SET);
-	t->flags |= SN_REDISP;
-	t->srv = NULL; /* it's left to the dispatcher to choose a server */
-	http_flush_cookie_flags(&t->txn);
+	/* it's left to the dispatcher to choose a server */
+	t->flags &= ~(SN_ASSIGNED | SN_ADDR_SET);
 	return 0;
 }
 
diff --git a/src/checks.c b/src/checks.c
index 124f40c..0557528 100644
--- a/src/checks.c
+++ b/src/checks.c


@@ -73,11 +73,9 @@ static int redistribute_pending(struct server *s)

 			sess->srv->redispatches++;
 			sess->be->redispatches++;
 
-			sess->flags &= ~(SN_DIRECT | SN_ASSIGNED | SN_ADDR_SET);
-			sess->flags |= SN_REDISP;
+			/* it's left to the dispatcher to choose a server */
+			sess->flags &= ~(SN_ASSIGNED | SN_ADDR_SET);
 
-			sess->srv = NULL; /* it's left to the dispatcher to choose a server */
-			http_flush_cookie_flags(&sess->txn);
 			pendconn_free(pc);
 			task_wakeup(sess->task);
 			xferred++;
diff --git a/src/proto_http.c b/src/proto_http.c
index 16b2f07..4e71535 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c


@@ -2536,10 +2536,8 @@ int process_srv(struct session *t)

 				}
 				t->be->redispatches++;
 
-				t->flags &= ~(SN_DIRECT | SN_ASSIGNED | SN_ADDR_SET);
-				t->flags |= SN_REDISP;
-				t->srv = NULL; /* it's left to the dispatcher to choose a server */
-				http_flush_cookie_flags(txn);
+				/* it's left to the dispatcher to choose a server */
+				t->flags &= ~(SN_ASSIGNED | SN_ADDR_SET);
 
 				/* first, get a connection */
 				if (srv_redispatch_connect(t))
-- 
1.5.3.7