[PATCH] Don't increment server connections too much + fix retries

From: Krzysztof Oledzki <ole#ans.pl>
Date: Mon, 04 Feb 2008 02:17:01 +0100

From 9af493edbe56489524f57c22d58aca0584931c8a Mon Sep 17 00:00:00 2001 From: Krzysztof Piotr Oledzki <ole#ans.pl> Date: Mon, 4 Feb 2008 02:10:09 +0100
Subject: [BUG] Don't increment server connections too much + fix retries

Commit 98937b875798e10fac671d109355cde29d2a411a while fixing one bug introduced another one. With "retries 4" and "option redispatch" haproxy tries to connect 4 times to one server server and 1 time to a second one. However logs showed 5 connections to the first server (the last one was counted twice) and 2 to the second.

This patch also fixes srv->retries and be->retries increments.

Now I get: 3 retries and 1 error in a first server (4 cum_sess) and 1 error in a second server (1 cum_sess) with:  retries 4
 option redispatch

and: 4 retries and 1 error (5 cum_sess) with:  retries 4

So, the number of connections effectively served by a server is:  srv->cum_sess - srv->failed_conns - srv->retries

Finally!

However, IMHO the fix from 98937b875798e10fac671d109355cde29d2a411a is wrong. Now be->cum_sess < SUM(servers->cum_sess) as with 1 sesssion processed by a backed I get 5 "sessions" processed by servers. This is very... abnormal. IMHO we should only increment srv->cum_sess once and only when a connection was successfully established but we need an additional variable: srv->conns.

So, if it is OK I will send an additional patch that renames "srv->cum_sess" into "srv->conns" and reintroduces "srv->cum_sess".

The final situation is going to look like this:

 be->cum_sess: connections (sessions) sent from a frontend to a backend
 be->failed_conns: unhandled connections (sessions) by a backend
 be->cum_sess - be->failed_conns: properly handled connections (sessions) by a backend

 srv->cum_sess: connections (sessions) successfully sent to a server

 srv->conns: connections sent to a server (both successfully and unsuccessfully)  srv->conns - srv->failed_conns - srv->retries: connections effectively served by a server

Please note that:

• SUM(servers->failed_conns) >= be->failed_conns as one session may be sent to two servers and both can be down
• SUM(servers->cum_sess) <= be->cum_sess as it is possible for a session to be unhandled when it was not possible to connect to a server

Comments?

---
 src/backend.c    |    5 -----
 src/proto_http.c |    5 ++++-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/backend.c b/src/backend.c
index b545e91..d2d5fe4 100644
--- a/src/backend.c
+++ b/src/backend.c


@@ -1354,9 +1354,6 @@ int srv_count_retry_down(struct session *t, int conn_err)

 {
 	/* we are in front of a retryable error */
 	t->conn_retries--;
-	if (t->srv)
-		t->srv->retries++;
-	t->be->retries++;
 
 	if (t->conn_retries < 0) {
 		/* if not retryable anymore, let's abort */


@@ -1364,8 +1361,6 @@ int srv_count_retry_down(struct session *t, int conn_err)

 		srv_close_with_err(t, conn_err, SN_FINST_C,
 				   503, error_message(t, HTTP_ERR_503));
 		if (t->srv)
-			t->srv->cum_sess++;
-		if (t->srv)
 			t->srv->failed_conns++;
 		t->be->failed_conns++;
 
diff --git a/src/proto_http.c b/src/proto_http.c
index 4e71535..e15ae92 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c


@@ -2530,7 +2530,6 @@ int process_srv(struct session *t)

 					task_wakeup(t->srv->queue_mgt);
 
 				if (t->srv) {
-					t->srv->cum_sess++;
 					t->srv->failed_conns++;
 					t->srv->redispatches++;
 				}


@@ -2542,6 +2541,10 @@ int process_srv(struct session *t)

 				/* first, get a connection */
 				if (srv_redispatch_connect(t))
 					return t->srv_state != SV_STCONN;
+			} else {
+				if (t->srv)
+					t->srv->retries++;
+				t->be->retries++;
 			}
 
 			do {
-- 
1.5.3.7

Received on 2008/02/04 02:17