On Mon, Feb 18, 2008 at 06:58:32PM +0300, Vsevolod Stakhov wrote:
 [...]
> >I've made some modifications to patch to attract your wishes, thought 
> >I've not tested it in production yet.
> 
> Sorry, attached reversed diff.
> 

Hi, and sorry about the delay!  Now that libevent 1.4 is stable, I'm
able to spend more time getting more patches into the next series.  I
did a match to make evdns.c threadsafe, but after I did so your patch
no longer applied cleanly.  I've done an updated version of your patch
and attached it here; could you test it out and let me know if it
works for you and does what you want under load?

You'll need to apply it to svn trunk; it won't work with 1.4.

I'm still curious whether evdns_transmit() shows up on your profiles:
it's the only remaining common case where we walk the list of all
inflight requests.  As you said before, your patch shouldn't make it
any slower, but I'm curious whether it appears slow for you at all.
If so, there should be another list of requests for which transmit_me
is set, so we don't need to walk the whole list of inflight requests
just in order to retry those.

yrs,
-- 
Nick
=== evdns.c
==================================================================
--- evdns.c	(revision 15216)
+++ evdns.c	(local)
@@ -293,8 +293,16 @@
 };
 
 struct evdns_base {
-	struct request *req_head, *req_waiting_head;
+	/* An array of n_req_heads circular lists for inflight requests.
+	 * Each inflight request req is in req_heads[req->trans_id % n_req_heads].
+	 */
+	struct request **req_heads;
+	/* A circular list of requests that we're waiting to send, but haven't
+	 * sent yet because there are too many requests inflight */
+	struct request *req_waiting_head;
+	/* A circular list of nameservers. */
 	struct nameserver *server_head;
+	int n_req_heads;
 
 	struct event_base *event_base;
 
@@ -330,6 +338,8 @@
 	((struct server_request*)											\
 	 (((char*)(base_ptr) - OFFSET_OF(struct server_request, base))))
 
+#define REQ_HEAD(base, id) ((base)->req_heads[id % (base)->n_req_heads])
+
 /* These are the timeout values for nameservers. If we find a nameserver is down */
 /* we try to probe it at intervals as given below. Values are in seconds. */
 static const struct timeval global_nameserver_timeouts[] = {{10, 0}, {60, 0}, {300, 0}, {900, 0}, {3600, 0}};
@@ -453,7 +463,8 @@
 /* failure */
 static struct request *
 request_find_from_trans_id(struct evdns_base *base, u16 trans_id) {
-	struct request *req = base->req_head, *const started_at = base->req_head;
+	struct request *req = REQ_HEAD(base, trans_id);
+	struct request *const started_at = req;
 
 	if (req) {
 		do {
@@ -511,6 +522,7 @@
 nameserver_failed(struct nameserver *const ns, const char *msg) {
 	struct request *req, *started_at;
 	struct evdns_base *base = ns->base;
+	int i;
 	/* if this nameserver has already been marked as failed */
 	/* then don't do anything */
 	if (!ns->state) return;
@@ -544,16 +556,18 @@
 	/* trying to reassign requests to one */
 	if (!base->global_good_nameservers) return;
 
-	req = started_at = base->req_head;
-	if (req) {
-		do {
-			if (req->tx_count == 0 && req->ns == ns) {
-				/* still waiting to go out, can be moved */
-				/* to another server */
-				req->ns = nameserver_pick(base);
-			}
-			req = req->next;
-		} while (req != started_at);
+	for (i = 0; i < base->n_req_heads; ++i) {
+		req = started_at = base->req_heads[i];
+		if (req) {
+			do {
+				if (req->tx_count == 0 && req->ns == ns) {
+					/* still waiting to go out, can be moved */
+					/* to another server */
+					req->ns = nameserver_pick(base);
+				}
+				req = req->next;
+			} while (req != started_at);
+		}
 	}
 }
 
@@ -665,7 +679,7 @@
 		req->ns = nameserver_pick(base);
 		request_trans_id_set(req, transaction_id_pick(base));
 
-		evdns_request_insert(req, &base->req_head);
+		evdns_request_insert(req, &REQ_HEAD(base, req->trans_id));
 		evdns_request_transmit(req);
 		evdns_transmit(base);
 	}
@@ -757,19 +771,19 @@
 				/* a new request was issued so this request is finished and */
 				/* the user callback will be made when that request (or a */
 				/* child of it) finishes. */
-				request_finished(req, &req->base->req_head);
+				request_finished(req, &REQ_HEAD(req->base, req->trans_id));
 				return;
 			}
 		}
 
 		/* all else failed. Pass the failure up */
 		reply_callback(req, 0, error, NULL);
-		request_finished(req, &req->base->req_head);
+		request_finished(req, &REQ_HEAD(req->base, req->trans_id));
 	} else {
 		/* all ok, tell the user */
 		reply_callback(req, ttl, 0, reply);
 		nameserver_up(req->ns);
-		request_finished(req, &req->base->req_head);
+		request_finished(req, &REQ_HEAD(req->base, req->trans_id));
 	}
 }
 
@@ -1106,20 +1120,12 @@
 static u16
 transaction_id_pick(struct evdns_base *base) {
 	for (;;) {
-		const struct request *req, *started_at;
 		u16 trans_id = trans_id_function();
 
 		if (trans_id == 0xffff) continue;
 		/* now check to see if that id is already inflight */
-		req = started_at = base->req_head;
-		if (req) {
-			do {
-				if (req->trans_id == trans_id) break;
-				req = req->next;
-			} while (req != started_at);
-		}
-		/* we didn't find it, so this is a good id */
-		if (req == started_at) return trans_id;
+		if (request_find_from_trans_id(base, trans_id) == NULL)
+			return trans_id;
 	}
 }
 
@@ -1900,7 +1906,7 @@
 	if (req->tx_count >= req->base->global_max_retransmits) {
 		/* this request has failed */
 		reply_callback(req, 0, DNS_ERR_TIMEOUT, NULL);
-		request_finished(req, &req->base->req_head);
+		request_finished(req, &REQ_HEAD(req->base, req->trans_id));
 	} else {
 		/* retransmit it */
 		evdns_request_transmit(req);
@@ -2015,18 +2021,21 @@
 static int
 evdns_transmit(struct evdns_base *base) {
 	char did_try_to_transmit = 0;
+	int i;
 
-	if (base->req_head) {
-		struct request *const started_at = base->req_head, *req = base->req_head;
-		/* first transmit all the requests which are currently waiting */
-		do {
-			if (req->transmit_me) {
-				did_try_to_transmit = 1;
-				evdns_request_transmit(req);
-			}
+	for (i = 0; i < base->n_req_heads; ++i) {
+		if (base->req_heads[i]) {
+			struct request *const started_at = base->req_heads[i], *req = started_at;
+			/* first transmit all the requests which are currently waiting */
+			do {
+				if (req->transmit_me) {
+					did_try_to_transmit = 1;
+					evdns_request_transmit(req);
+				}
 
-			req = req->next;
-		} while (req != started_at);
+				req = req->next;
+			} while (req != started_at);
+		}
 	}
 
 	return did_try_to_transmit;
@@ -2058,7 +2067,7 @@
 evdns_base_clear_nameservers_and_suspend(struct evdns_base *base)
 {
 	struct nameserver *server = base->server_head, *started_at = base->server_head;
-	struct request *req = base->req_head, *req_started_at = base->req_head;
+	int i;
 
 	if (!server)
 		return 0;
@@ -2077,28 +2086,33 @@
 	base->server_head = NULL;
 	base->global_good_nameservers = 0;
 
-	while (req) {
-		struct request *next = req->next;
-		req->tx_count = req->reissue_count = 0;
-		req->ns = NULL;
-		/* ???? What to do about searches? */
-		(void) evtimer_del(&req->timeout_event);
-		req->trans_id = 0;
-		req->transmit_me = 0;
+	for (i = 0; i < base->n_req_heads; ++i) {
+		struct request *req, *req_started_at;
+		req = req_started_at = base->req_heads[i];
+		while (req) {
+			struct request *next = req->next;
+			req->tx_count = req->reissue_count = 0;
+			req->ns = NULL;
+			/* ???? What to do about searches? */
+			(void) evtimer_del(&req->timeout_event);
+			req->trans_id = 0;
+			req->transmit_me = 0;
 
-		base->global_requests_waiting++;
-		evdns_request_insert(req, &base->req_waiting_head);
-		/* We want to insert these suspended elements at the front of
-		 * the waiting queue, since they were pending before any of
-		 * the waiting entries were added.  This is a circular list,
-		 * so we can just shift the start back by one.*/
-		base->req_waiting_head = base->req_waiting_head->prev;
+			base->global_requests_waiting++;
+			evdns_request_insert(req, &base->req_waiting_head);
+			/* We want to insert these suspended elements at the front of
+			 * the waiting queue, since they were pending before any of
+			 * the waiting entries were added.  This is a circular list,
+			 * so we can just shift the start back by one.*/
+			base->req_waiting_head = base->req_waiting_head->prev;
 
-		if (next == req_started_at)
-			break;
-		req = next;
+			if (next == req_started_at)
+				break;
+			req = next;
+		}
+		base->req_heads[i] = NULL;
 	}
-	base->req_head = NULL;
+
 	base->global_requests_inflight = 0;
 
 	return 0;
@@ -2317,7 +2331,7 @@
 	if (req->ns) {
 		/* if it has a nameserver assigned then this is going */
 		/* straight into the inflight queue */
-		evdns_request_insert(req, &base->req_head);
+		evdns_request_insert(req, &REQ_HEAD(base, req->trans_id));
 		base->global_requests_inflight++;
 		evdns_request_transmit(req);
 	} else {
@@ -2735,6 +2749,43 @@
 		return r;
 }
 
+static int
+evdns_base_set_max_requests_inflight(struct evdns_base *base, int maxinflight)
+{
+	int old_n_heads = base->n_req_heads, n_heads;
+	struct request **old_heads = base->req_heads, **new_heads, *req;
+	int i;
+	if (maxinflight < 1)
+		maxinflight = 1;
+	n_heads = (maxinflight+4) / 5;
+	assert(n_heads > 0);
+	new_heads = event_malloc(n_heads * sizeof(struct request*));
+	if (!new_heads)
+		return (-1);
+	for (i=0; i < n_heads; ++i)
+		new_heads[i] = NULL;
+	if (old_heads) {
+		for (i = 0; i < old_n_heads; ++i) {
+			while (old_heads[i]) {
+				req = old_heads[i];
+				if (req->next == req) {
+					old_heads[i] = NULL;
+				} else {
+					old_heads[i] = req->next;
+					req->next->prev = req->prev;
+					req->prev->next = req->next;
+				}
+				evdns_request_insert(req, &new_heads[req->trans_id % n_heads]);
+			}
+		}
+		event_free(old_heads);
+	}
+	base->req_heads = new_heads;
+	base->n_req_heads = n_heads;
+	base->global_max_requests_inflight = maxinflight;
+	return (0);
+}
+
 /* exported function */
 int
 evdns_base_set_option(struct evdns_base *base,
@@ -2767,7 +2818,7 @@
 		if (!(flags & DNS_OPTION_MISC)) return 0;
 		log(EVDNS_LOG_DEBUG, "Setting maximum inflight requests to %d",
 			maxinflight);
-		base->global_max_requests_inflight = maxinflight;
+		evdns_base_set_max_requests_inflight(base, maxinflight);
 	} else if (!strncmp(option, "attempts:", 9)) {
 		int retries = strtoint(val);
 		if (retries == -1) return -1;
@@ -3105,12 +3156,17 @@
 	if (base == NULL)
 		return (NULL);
 	memset(base, 0, sizeof(struct evdns_base));
-	base->req_head = base->req_waiting_head = NULL;
+	base->req_waiting_head = NULL;
+
+	/* Set max requests inflight and allocate req_heads. */
+	base->req_heads = NULL;
+	evdns_base_set_max_requests_inflight(base, 64);
+
 	base->server_head = NULL;
 	base->event_base = event_base;
 	base->global_good_nameservers = base->global_requests_inflight =
 		base->global_requests_waiting = 0;
-	base->global_max_requests_inflight = 64;
+
 	base->global_timeout.tv_sec = 5;
 	base->global_timeout.tv_usec = 0;
 	base->global_max_reissues = 1;
@@ -3168,11 +3224,14 @@
 {
 	struct nameserver *server, *server_next;
 	struct search_domain *dom, *dom_next;
+	int i;
 
-	while (base->req_head) {
-		if (fail_requests)
-			reply_callback(base->req_head, 0, DNS_ERR_SHUTDOWN, NULL);
-		request_finished(base->req_head, &base->req_head);
+	for (i = 0; i < base->n_req_heads; ++i) {
+		while (base->req_heads[i]) {
+			if (fail_requests)
+				reply_callback(base->req_heads[i], 0, DNS_ERR_SHUTDOWN, NULL);
+			request_finished(base->req_heads[i], &REQ_HEAD(base, base->req_heads[i]->trans_id));
+		}
 	}
 	while (base->req_waiting_head) {
 		if (fail_requests)
_______________________________________________
Libevent-users mailing list
Libevent-users@monkey.org
http://monkeymail.org/mailman/listinfo/libevent-users

Reply via email to