dovecot-2.2: lib-http: Avoid hanging on urgent requests.

dovecot at dovecot.org dovecot at dovecot.org
Tue Mar 5 21:44:22 EET 2013


details:   http://hg.dovecot.org/dovecot-2.2/rev/52e5d4186006
changeset: 16001:52e5d4186006
user:      Timo Sirainen <tss at iki.fi>
date:      Tue Mar 05 21:44:07 2013 +0200
description:
lib-http: Avoid hanging on urgent requests.
Patch by Timo & Stephan. There are still some problems though, all urgent
requests don't seem to get a new connection.

diffstat:

 src/lib-http/http-client-connection.c |   1 +
 src/lib-http/http-client-host.c       |  60 +++++++++++-----------
 src/lib-http/http-client-peer.c       |  90 +++++++++++++++++++---------------
 src/lib-http/http-client-private.h    |   7 +-
 4 files changed, 82 insertions(+), 76 deletions(-)

diffs (truncated from 352 to 300 lines):

diff -r 00ee7a8306c7 -r 52e5d4186006 src/lib-http/http-client-connection.c
--- a/src/lib-http/http-client-connection.c	Tue Mar 05 16:32:12 2013 +0200
+++ b/src/lib-http/http-client-connection.c	Tue Mar 05 21:44:07 2013 +0200
@@ -787,6 +787,7 @@
 
 	http_client_connection_debug(conn, "Connection destroy");
 
+	conn->closing = TRUE;
 	conn->connected = FALSE;
 
 #ifdef HTTP_BUILD_SSL
diff -r 00ee7a8306c7 -r 52e5d4186006 src/lib-http/http-client-host.c
--- a/src/lib-http/http-client-host.c	Tue Mar 05 16:32:12 2013 +0200
+++ b/src/lib-http/http-client-host.c	Tue Mar 05 21:44:07 2013 +0200
@@ -68,7 +68,6 @@
 		hport->ssl = ssl;
 		hport->ips_connect_idx = 0;
 		i_array_init(&hport->request_queue, 16);
-		i_array_init(&hport->urgent_request_queue, 4);
 	}
 
 	return hport;
@@ -83,11 +82,7 @@
 	array_foreach_modifiable(&hport->request_queue, req) {
 		http_client_request_error(*req, status, error);
 	}
-	array_foreach_modifiable(&hport->urgent_request_queue, req) {
-		http_client_request_error(*req, status, error);
-	}
 	array_clear(&hport->request_queue);
-	array_clear(&hport->urgent_request_queue);
 }
 
 static void http_client_host_port_deinit(struct http_client_host_port *hport)
@@ -95,7 +90,6 @@
 	http_client_host_port_error
 		(hport, HTTP_CLIENT_REQUEST_ERROR_ABORTED, "Aborted");
 	array_free(&hport->request_queue);
-	array_free(&hport->urgent_request_queue);
 }
 
 static void
@@ -103,14 +97,12 @@
 	struct http_client_request *req)
 {
 	struct http_client_request **req_idx;
-	ARRAY_TYPE(http_client_request) *queue =
-		(req->urgent ? &hport->urgent_request_queue : &hport->request_queue);
 	unsigned int idx;
 
-	array_foreach_modifiable(queue, req_idx) {
+	array_foreach_modifiable(&hport->request_queue, req_idx) {
 		if (*req_idx == req) {
-			idx = array_foreach_idx(queue, req_idx);
-			array_delete(queue, idx, 1);
+			idx = array_foreach_idx(&hport->request_queue, req_idx);
+			array_delete(&hport->request_queue, idx, 1);
 			break;
 		}
 	}
@@ -297,7 +289,7 @@
 	/* add request to host (grouped by tcp port) */
 	hport = http_client_host_port_init(host, req->port, req->ssl);
 	if (req->urgent)
-		array_append(&hport->urgent_request_queue, &req, 1);
+		array_insert(&hport->request_queue, 0, &req, 1);
 	else
 		array_append(&hport->request_queue, &req, 1);
 
@@ -316,23 +308,26 @@
 	const struct http_client_peer_addr *addr, bool no_urgent)
 {
 	struct http_client_host_port *hport;
-	struct http_client_request *const *req_idx;
+	struct http_client_request *const *requests;
 	struct http_client_request *req;
+	unsigned int i, count;
 
 	hport = http_client_host_port_find(host, addr->port, addr->ssl);
 	if (hport == NULL)
 		return NULL;
-	if (!no_urgent && array_count(&hport->urgent_request_queue) > 0) {
-		req_idx = array_idx(&hport->urgent_request_queue, 0);
-		req = *req_idx;
-		array_delete(&hport->urgent_request_queue, 0, 1);
-	} else if (array_count(&hport->request_queue) > 0) {
-		req_idx = array_idx(&hport->request_queue, 0);
-		req = *req_idx;
-		array_delete(&hport->request_queue, 0, 1);
-	} else {
+
+	requests = array_get(&hport->request_queue, &count);
+	if (count == 0)
 		return NULL;
+	i = 0;
+	if (requests[0]->urgent && no_urgent) {
+		for (; requests[i]->urgent; i++) {
+			if (i == count)
+				return NULL;
+		}
 	}
+	req = requests[i];
+	array_delete(&hport->request_queue, i, 1);
 
 	http_client_host_debug(host,
 		"Connection to peer %s:%u claimed request %s %s",
@@ -342,18 +337,23 @@
 	return req;
 }
 
-bool http_client_host_have_requests(struct http_client_host *host,
-	const struct http_client_peer_addr *addr, bool urgent)
+unsigned int http_client_host_requests_pending(struct http_client_host *host,
+	const struct http_client_peer_addr *addr, unsigned int *num_urgent_r)
 {
 	struct http_client_host_port *hport;
+	struct http_client_request *const *requests;
+	unsigned int count, i;
+
+	*num_urgent_r = 0;
 
 	hport = http_client_host_port_find(host, addr->port, addr->ssl);
 	if (hport == NULL)
-		return FALSE;
-	
-	if (urgent)
-		return (array_count(&hport->urgent_request_queue) > 0);
-	return (array_count(&hport->request_queue) > 0);
+		return 0;
+
+	requests = array_get(&hport->request_queue, &count);
+	for (i = 0; i < count && requests[i]->urgent; i++)
+		(*num_urgent_r)++;
+	return count;
 }
 
 void http_client_host_drop_request(struct http_client_host *host,
@@ -364,7 +364,7 @@
 	hport = http_client_host_port_find(host, req->port, req->ssl);
 	if (hport == NULL)
 		return;
-	
+
 	http_client_host_port_drop_request(hport, req);
 }
 
diff -r 00ee7a8306c7 -r 52e5d4186006 src/lib-http/http-client-peer.c
--- a/src/lib-http/http-client-peer.c	Tue Mar 05 16:32:12 2013 +0200
+++ b/src/lib-http/http-client-peer.c	Tue Mar 05 21:44:07 2013 +0200
@@ -91,42 +91,51 @@
 }
 
 static int
-http_client_peer_connect(struct http_client_peer *peer)
+http_client_peer_connect(struct http_client_peer *peer, unsigned int count)
 {
 	struct http_client_connection *conn;
+	unsigned int i;
 
-	conn = http_client_connection_create(peer);
-	if (conn == NULL) {
-		http_client_peer_debug(peer, "Failed to make new connection");
-		return -1;
+	for (i = 0; i < count; i++) {
+		http_client_peer_debug(peer, "Making new connection %u of %u", i+1, count);
+
+		conn = http_client_connection_create(peer);
+		if (conn == NULL) {
+			http_client_peer_debug(peer, "Failed to make new connection");
+			return -1;
+		}
 	}
 
 	return 0;
 }
 
-static bool
-http_client_peer_requests_pending(struct http_client_peer *peer, bool urgent)
+static unsigned int
+http_client_peer_requests_pending(struct http_client_peer *peer, unsigned int *num_urgent_r)
 {
 	struct http_client_host *const *host;
+	unsigned int num_requests = 0, num_urgent = 0, requests, urgent;
 
 	array_foreach(&peer->hosts, host) {
-		if (http_client_host_have_requests(*host, &peer->addr, urgent))
-			return TRUE;
+		requests = http_client_host_requests_pending(*host, &peer->addr, &urgent);
+
+		num_requests += requests;
+		num_urgent += urgent;
 	}
-
-	return FALSE;
+	*num_urgent_r = num_urgent;
+	return num_requests;
 }
 
 static bool
-http_client_peer_next_request(struct http_client_peer *peer,
-	bool urgent)
+http_client_peer_next_request(struct http_client_peer *peer)
 {
 	struct http_client_connection *const *conn_idx;
 	struct http_client_connection *conn = NULL;
-	unsigned int closing = 0, min_waiting = UINT_MAX;
-	
+	unsigned int connecting = 0, closing = 0, min_waiting = UINT_MAX;
+	unsigned int num_urgent, new_connections;
+
 	/* at this point we already know that a request for this peer is pending
 	 */
+	(void)http_client_peer_requests_pending(peer, &num_urgent);
 
 	/* find the least busy connection */
 	array_foreach(&peer->conns, conn_idx) {
@@ -135,23 +144,27 @@
 			if (waiting < min_waiting) {
 				min_waiting = waiting;
 				conn = *conn_idx;
-				if (min_waiting == 0)
+				if (min_waiting == 0) {
+					/* found idle connection, use it now */
 					break;
+				}
 			}
 		}
-		/* count the number of closing connections */
+		/* count the number of connecting and closing connections */
 		if ((*conn_idx)->closing)
 			closing++;
+		else if (!(*conn_idx)->connected)
+			connecting++;
 	}
 
-	/* do we have an idle connection? */
+	/* did we find an idle connection? */
 	if (conn != NULL && min_waiting == 0) {
-		/* yes */
+		/* yes, use it */
 		return http_client_connection_next_request(conn);
 	}
 
 	/* no, but can we create a new connection? */		
-	if (!urgent && (array_count(&peer->conns) - closing) >=
+	if (num_urgent == 0 && (array_count(&peer->conns) - closing) >=
 		peer->client->set.max_parallel_connections) {
 		/* no */
 		if (conn == NULL)
@@ -160,8 +173,13 @@
 		return http_client_connection_next_request(conn);
 	}
 
-	/* yes */
-	if (http_client_peer_connect(peer) < 0) {
+	/* yes, determine how many connections to set up */
+	if (num_urgent == 0) {
+		new_connections = 1;
+	} else {
+		new_connections = (num_urgent > connecting ? num_urgent - connecting : 0);
+	}
+	if (http_client_peer_connect(peer, new_connections) < 0) {
 		/* connection failed */
 		if (conn == NULL)
 			return FALSE;
@@ -175,17 +193,7 @@
 
 void http_client_peer_handle_requests(struct http_client_peer *peer)
 {
-	/* check urgent requests first */
-	while (http_client_peer_requests_pending(peer, TRUE)) {
-		if (!http_client_peer_next_request(peer, TRUE))
-			break;
-	}
-
-	/* check normal requests once we're done */
-	while (http_client_peer_requests_pending(peer, FALSE)) {
-		if (!http_client_peer_next_request(peer, FALSE))
-			break;
-	}
+	while (http_client_peer_next_request(peer)) ;
 }
 
 static struct http_client_peer *
@@ -234,7 +242,7 @@
 	}
 #endif
 
-	if (http_client_peer_connect(peer) < 0) {
+	if (http_client_peer_connect(peer, 1) < 0) {
 		http_client_peer_free(&peer);
 		return NULL;
 	}
@@ -308,7 +316,7 @@
 	if (!exists)
 		array_append(&peer->hosts, &host, 1);
 	if (exists || array_count(&peer->hosts) > 1)
-		(void)http_client_peer_next_request(peer, FALSE);
+		(void)http_client_peer_next_request(peer);
 }


More information about the dovecot-cvs mailing list