Vsevolod Stakhov wrote:
[EMAIL PROTECTED] wrote:
On Fri, Feb 15, 2008 at 07:22:34PM +0300, Vsevolod Stakhov wrote:
Hi,
We found a problem with list requests_inflight while using evdns on
highly loaded system (about 2000 DNS requests per second). First of
all linear search is slow and second is that this list is not big
enough to handle all requests, so many requests were in
requests_waiting list, that has no limits on length. Due to this
application grows in size constantly and is not able to serve
requests. I've replaced linear list with hashed list, that improves
performance.
Hi! This looks pretty good. I'd like to apply it for trunk to apply
it to the next release after 1.4. (I want to avoid applying anything
but straight bugfixes to 1.4 at this point, so that it can get out of
beta soon.) A few comments and questions:
>> ...
I've made some modifications to patch to attract your wishes, thought
I've not tested it in production yet.
diff -r 2609c564f854 -r d3e5ec3459f9 evdns.c
--- a/evdns.c Mon Feb 18 18:55:03 2008 +0300
+++ b/evdns.c Wed Feb 13 15:31:39 2008 +0300
@@ -231,7 +231,7 @@ struct nameserver {
char write_waiting; /* true if we are waiting for EV_WRITE events */
};
-static struct request **req_head = NULL, *req_waiting_head = NULL;
+static struct request *req_head = NULL, *req_waiting_head = NULL;
static struct nameserver *server_head = NULL;
/* Represents a local port where we're listening for DNS requests. Right now,
*/
@@ -310,9 +310,7 @@ static int global_requests_inflight = 0;
/* and are counted here */
static int global_requests_waiting = 0;
-static int global_max_requests_inflight = 1024;
-/* numbers of nests in hashed list of pending requests */
-static int global_max_nests = 128;
+static int global_max_requests_inflight = 64;
static struct timeval global_timeout = {5, 0}; /* 5 seconds */
static int global_max_reissues = 1; /* a reissue occurs when we get some
errors from the server */
@@ -445,16 +443,13 @@ _evdns_log(int warn, const char *fmt, ..
/* failure */
static struct request *
request_find_from_trans_id(u16 trans_id) {
- struct request *req, *started_at;
- if (req_head) {
- req = req_head[trans_id % global_max_nests];
- started_at = req_head[trans_id % global_max_nests];
- if (req) {
- do {
- if (req->trans_id == trans_id) return req;
- req = req->next;
- } while (req != started_at);
- }
+ struct request *req = req_head, *const started_at = req_head;
+
+ if (req) {
+ do {
+ if (req->trans_id == trans_id) return req;
+ req = req->next;
+ } while (req != started_at);
}
return NULL;
@@ -503,7 +498,6 @@ static void
static void
nameserver_failed(struct nameserver *const ns, const char *msg) {
struct request *req, *started_at;
- int i;
/* if this nameserver has already been marked as failed */
/* then don't do anything */
if (!ns->state) return;
@@ -533,21 +527,19 @@ nameserver_failed(struct nameserver *con
/* if we don't have *any* good nameservers then there's no point */
/* trying to reassign requests to one */
- if (!global_good_nameservers || !req_head) return;
-
- for (i = 0; i < global_max_nests; i ++) {
- req = req_head[i];
- started_at = req_head[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();
- }
- req = req->next;
- } while (req != started_at);
- }
+ if (!global_good_nameservers) return;
+
+ req = req_head;
+ started_at = 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();
+ }
+ req = req->next;
+ } while (req != started_at);
}
}
@@ -657,10 +649,8 @@ evdns_requests_pump_waiting_queue(void)
req->ns = nameserver_pick();
request_trans_id_set(req, transaction_id_pick());
-
- if (req_head) {
- evdns_request_insert(req, &req_head[req->trans_id %
global_max_nests]);
- }
+
+ evdns_request_insert(req, &req_head);
evdns_request_transmit(req);
evdns_transmit();
}
@@ -752,25 +742,19 @@ reply_handle(struct request *const req,
/* 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. */
- if (req_head) {
- request_finished(req,
&req_head[req->trans_id % global_max_nests]);
- }
+ request_finished(req, &req_head);
return;
}
}
/* all else failed. Pass the failure up */
reply_callback(req, 0, error, NULL);
- if (req_head) {
- request_finished(req, &req_head[req->trans_id %
global_max_nests]);
- }
+ request_finished(req, &req_head);
} else {
/* all ok, tell the user */
reply_callback(req, ttl, 0, reply);
nameserver_up(req->ns);
- if (req_head) {
- request_finished(req, &req_head[req->trans_id %
global_max_nests]);
- }
+ request_finished(req, &req_head);
}
}
@@ -1106,14 +1090,12 @@ static u16
static u16
transaction_id_pick(void) {
for (;;) {
- const struct request *req = NULL, *started_at = NULL;
+ const struct request *req = req_head, *started_at;
u16 trans_id = trans_id_function();
if (trans_id == 0xffff) continue;
/* now check to see if that id is already inflight */
- if (req_head) {
- req = started_at = req_head[trans_id %
global_max_nests];
- }
+ req = started_at = req_head;
if (req) {
do {
if (req->trans_id == trans_id) break;
@@ -1887,9 +1869,7 @@ evdns_request_timeout_callback(int fd, s
if (req->tx_count >= global_max_retransmits) {
/* this request has failed */
reply_callback(req, 0, DNS_ERR_TIMEOUT, NULL);
- if (req_head) {
- request_finished(req, &req_head[req->trans_id %
global_max_nests]);
- }
+ request_finished(req, &req_head);
} else {
/* retransmit it */
evdns_request_transmit(req);
@@ -2001,22 +1981,19 @@ nameserver_send_probe(struct nameserver
/* 1 tried to transmit something */
static int
evdns_transmit(void) {
- int i;
char did_try_to_transmit = 0;
-
- for (i = 0; i < global_max_nests; i ++) {
- if (req_head && req_head[i]) {
- struct request *const started_at = req_head[i], *req =
req_head[i];
- /* 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);
- }
+
+ if (req_head) {
+ struct request *const started_at = req_head, *req = 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);
+ }
+
+ req = req->next;
+ } while (req != started_at);
}
return did_try_to_transmit;
@@ -2042,8 +2019,7 @@ evdns_clear_nameservers_and_suspend(void
evdns_clear_nameservers_and_suspend(void)
{
struct nameserver *server = server_head, *started_at = server_head;
- struct request *req, *req_started_at;
- int i;
+ struct request *req = req_head, *req_started_at = req_head;
if (!server)
return 0;
@@ -2060,37 +2036,29 @@ evdns_clear_nameservers_and_suspend(void
}
server_head = NULL;
global_good_nameservers = 0;
-
- if (req_head) {
- for (i = 0; i < global_max_nests; i ++) {
- req = req_head[i];
- req_started_at = req;
-
- 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;
-
- global_requests_waiting++;
- evdns_request_insert(req, &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.*/
- req_waiting_head = req_waiting_head->prev;
-
- if (next == req_started_at)
- break;
- req = next;
- }
- req_head[i] = NULL;
- }
- }
-
+
+ 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;
+
+ global_requests_waiting++;
+ evdns_request_insert(req, &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.*/
+ req_waiting_head = req_waiting_head->prev;
+
+ if (next == req_started_at)
+ break;
+ req = next;
+ }
+ req_head = NULL;
global_requests_inflight = 0;
return 0;
@@ -2277,9 +2245,7 @@ request_submit(struct request *const req
if (req->ns) {
/* if it has a nameserver assigned then this is going */
/* straight into the inflight queue */
- if (req_head) {
- evdns_request_insert(req, &req_head[req->trans_id %
global_max_nests]);
- }
+ evdns_request_insert(req, &req_head);
global_requests_inflight++;
evdns_request_transmit(req);
} else {
@@ -2681,9 +2647,6 @@ evdns_set_option(const char *option, con
log(EVDNS_LOG_DEBUG, "Setting maximum inflight requests to %d",
maxinflight);
global_max_requests_inflight = maxinflight;
- global_max_nests = global_max_requests_inflight / 10;
- req_head = realloc (req_head, global_max_nests * sizeof
(req_head[0]));
- if (req_head == NULL) return -1;
} else if (!strncmp(option, "attempts:", 9)) {
int retries = strtoint(val);
if (retries == -1) return -1;
@@ -2751,7 +2714,7 @@ evdns_resolv_conf_parse(int flags, const
u8 *resolv;
char *start;
int err = 0;
-
+
log(EVDNS_LOG_DEBUG, "Parsing resolv.conf file %s", filename);
fd = open(filename, O_RDONLY);
@@ -3003,28 +2966,9 @@ evdns_config_windows_nameservers(void)
#endif
int
-evdns_requests_list_init(void)
-{
- if (req_head == NULL) {
- req_head = malloc (global_max_nests * sizeof (req_head[0]));
- if (req_head == NULL) {
- return 4;
- }
- memset (req_head, 0, global_max_nests * sizeof (req_head[0]));
- }
-
- return 0;
-}
-
-int
evdns_init(void)
{
int res = 0;
-
- res = evdns_requests_list_init();
- if (res != 0) {
- return res;
- }
#ifdef WIN32
res = evdns_config_windows_nameservers();
#else
@@ -3058,14 +3002,11 @@ evdns_shutdown(int fail_requests)
{
struct nameserver *server, *server_next;
struct search_domain *dom, *dom_next;
- int i;
-
- for (i = 0; i < global_max_nests; i ++) {
- while (req_head && req_head[i]) {
- if (fail_requests)
- reply_callback(req_head[i], 0,
DNS_ERR_SHUTDOWN, NULL);
- request_finished(req_head[i], &req_head[i]);
- }
+
+ while (req_head) {
+ if (fail_requests)
+ reply_callback(req_head, 0, DNS_ERR_SHUTDOWN, NULL);
+ request_finished(req_head, &req_head);
}
while (req_waiting_head) {
if (fail_requests)
diff -r 2609c564f854 -r d3e5ec3459f9 http.c
--- a/http.c Mon Feb 18 18:55:03 2008 +0300
+++ b/http.c Wed Feb 13 15:31:39 2008 +0300
@@ -78,9 +78,6 @@
#ifdef HAVE_FCNTL_H
#include <fcntl.h>
#endif
-#ifndef WIN32
-#include <sys/un.h>
-#endif
#undef timeout_pending
#undef timeout_initialized
@@ -2352,25 +2349,18 @@ name_from_addr(struct sockaddr *sa, sock
static char ntop[NI_MAXHOST];
static char strport[NI_MAXSERV];
int ni_result;
-
- if (sa->sa_family == PF_UNIX) {
- strncpy (ntop, ((struct sockaddr_un *)sa)->sun_path, NI_MAXHOST
- 1);
- strport[0] = '0';
- strport[1] = '\0';
- }
- else {
- if ((ni_result = getnameinfo(sa, salen,
- ntop, sizeof(ntop), strport, sizeof(strport),
- NI_NUMERICHOST|NI_NUMERICSERV)) != 0) {
- if (ni_result == EAI_SYSTEM)
- event_err(1, "getnameinfo failed");
- else
- event_errx(1, "getnameinfo failed: %s",
gai_strerror(ni_result));
- }
- }
+
+ if ((ni_result = getnameinfo(sa, salen,
+ ntop, sizeof(ntop), strport, sizeof(strport),
+ NI_NUMERICHOST|NI_NUMERICSERV)) != 0) {
+ if (ni_result == EAI_SYSTEM)
+ event_err(1, "getnameinfo failed");
+ else
+ event_errx(1, "getnameinfo failed: %s",
gai_strerror(ni_result));
+ }
+
*phost = ntop;
*pport = strport;
-
#else
/* XXXX */
#endif
diff -r 2609c564f854 -r d3e5ec3459f9 test/regress_dns.c
--- a/test/regress_dns.c Mon Feb 18 18:55:03 2008 +0300
+++ b/test/regress_dns.c Wed Feb 13 15:31:39 2008 +0300
@@ -67,7 +67,6 @@ static int dns_err = 0;
static int dns_err = 0;
void dns_suite(void);
-int evdns_requests_list_init(void);
static void
dns_gethostbyname_cb(int result, char type, int count, int ttl,
@@ -304,11 +303,6 @@ dns_server(void)
dns_ok = 1;
fprintf(stdout, "DNS server support: ");
- if (evdns_requests_list_init() != 0) {
- fprintf(stdout, "Couldn't init dns requests hash list.\n");
- exit(1);
- }
-
/* Add ourself as the only nameserver, and make sure we really are
* the only nameserver. */
evdns_nameserver_ip_add("127.0.0.1:35353");
@@ -372,8 +366,8 @@ dns_suite(void)
dns_suite(void)
{
dns_server(); /* Do this before we call evdns_init. */
+
evdns_init();
-
dns_gethostbyname();
dns_gethostbyname6();
dns_gethostbyaddr();
_______________________________________________
Libevent-users mailing list
Libevent-users@monkey.org
http://monkeymail.org/mailman/listinfo/libevent-users
