From 83514326a33e2585733f4794902946c561930321 Mon Sep 17 00:00:00 2001 From: "Alan T. DeKok" Date: Sat, 11 Apr 2009 09:32:39 +0200 Subject: [PATCH] Fixed detail file handler to not go crazy In short, the detail timer events are now inserted with a saved fr_event_t. This allows *existing* timer events to be deleted when a new one is added. The previous code would *add* timer events on top of the existing ones, causing geometric increases in the number of polls per second. Also, re-arranged the detail && listener code so that there's only one location where the timer gets inserted, and only one location where the delays get propogated from the detail to the event handlers --- src/include/detail.h | 2 -- src/main/detail.c | 100 +++++++++++++++++++++++++++++---------------------- src/main/event.c | 88 +++++++++++++++------------------------------ 3 files changed, 87 insertions(+), 103 deletions(-) diff --git a/src/include/detail.h b/src/include/detail.h index 0ee4cb1..699b1be 100644 --- a/src/include/detail.h +++ b/src/include/detail.h @@ -18,7 +18,5 @@ int detail_print(rad_listen_t *this, char *buffer, size_t bufsize); int detail_encode(UNUSED rad_listen_t *this, UNUSED REQUEST *request); int detail_decode(UNUSED rad_listen_t *this, UNUSED REQUEST *request); int detail_parse(CONF_SECTION *cs, rad_listen_t *this); -int detail_delay(rad_listen_t *this); -int detail_poll_interval(rad_listen_t *listener); #endif /* DETAIL_H */ diff --git a/src/main/detail.c b/src/main/detail.c index f582817..3bb97bd 100644 --- a/src/main/detail.c +++ b/src/main/detail.c @@ -43,18 +43,31 @@ RCSID("$Id$") #define USEC (1000000) +typedef enum detail_state_t { + STATE_UNOPENED = 0, + STATE_UNLOCKED, + STATE_HEADER, + STATE_READING, + STATE_QUEUED, + STATE_RUNNING, + STATE_NO_REPLY, + STATE_REPLIED +} detail_state_t; + typedef struct listen_detail_t { - int delay_time; /* should be first entry */ + fr_event_t *ev; /* has to be first entry (ugh) */ + int delay_time; char *filename; char *filename_work; VALUE_PAIR *vps; FILE *fp; - int state; + detail_state_t state; time_t timestamp; fr_ipaddr_t client_ip; int load_factor; /* 1..100 */ int signal; int poll_interval; + int retry_interval; int has_rtt; int srtt; @@ -64,15 +77,6 @@ typedef struct listen_detail_t { } listen_detail_t; -#define STATE_UNOPENED (0) -#define STATE_UNLOCKED (1) -#define STATE_HEADER (2) -#define STATE_READING (3) -#define STATE_QUEUED (4) -#define STATE_RUNNING (5) -#define STATE_NO_REPLY (6) -#define STATE_REPLIED (7) - /* * If we're limiting outstanding packets, then mark the response * as being sent. @@ -91,7 +95,7 @@ int detail_send(rad_listen_t *listener, REQUEST *request) * caller it's OK to read more "detail" file stuff. */ if (request->reply->code == 0) { - data->delay_time = USEC; + data->delay_time = data->retry_interval * USEC; data->signal = 1; data->state = STATE_NO_REPLY; radius_signal_self(RADIUS_SIGNAL_SELF_DETAIL); @@ -177,17 +181,6 @@ int detail_send(rad_listen_t *listener, REQUEST *request) return 0; } -int detail_delay(rad_listen_t *listener) -{ - listen_detail_t *data = listener->data; - - if (!data->signal) return 0; - - data->signal = 0; - - return data->delay_time; -} - /* * Open the detail file, if we can. @@ -326,6 +319,12 @@ int detail_recv(rad_listen_t *listener, char buffer[2048]; listen_detail_t *data = listener->data; + /* + * We may be in the main thread. It needs to update the + * timers before we try to read from the file again. + */ + if (data->signal) return 0; + switch (data->state) { case STATE_UNOPENED: open_file: @@ -379,6 +378,7 @@ int detail_recv(rad_listen_t *listener, * Look for the header */ data->state = STATE_HEADER; + data->delay_time = USEC; data->vps = NULL; /* FALL-THROUGH */ @@ -728,20 +728,41 @@ int detail_print(rad_listen_t *this, char *buffer, size_t bufsize) this->server); } -int detail_encode(UNUSED rad_listen_t *this, UNUSED REQUEST *request) +/* + * Overloaded to return delay times. + */ +int detail_encode(rad_listen_t *this, UNUSED REQUEST *request) { + listen_detail_t *data = this->data; + /* - * We never encode responses "sent to" the detail file. + * We haven't sent a packet... delay things a bit. */ - return 0; + if (!data->signal) { + int delay = (data->poll_interval - 1) * USEC; + + /* + * Add +/- 0.25s of jitter + */ + delay += (USEC * 3) / 4; + delay += fr_rand() % (USEC / 2); + return delay; + } + + data->signal = 0; + + return data->delay_time; } -int detail_decode(UNUSED rad_listen_t *this, UNUSED REQUEST *request) + +/* + * Overloaded to return "should we fix delay times" + */ +int detail_decode(rad_listen_t *this, UNUSED REQUEST *request) { - /* - * We never decode responses read from the detail file. - */ - return 0; + listen_detail_t *data = this->data; + + return data->signal; } @@ -752,6 +773,8 @@ static const CONF_PARSER detail_config[] = { offsetof(listen_detail_t, load_factor), NULL, Stringify(10)}, { "poll_interval", PW_TYPE_INTEGER, offsetof(listen_detail_t, poll_interval), NULL, Stringify(1)}, + { "retry_interval", PW_TYPE_INTEGER, + offsetof(listen_detail_t, retry_interval), NULL, Stringify(30)}, { NULL, -1, 0, NULL, NULL } /* end the list */ }; @@ -773,7 +796,6 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this) } data = this->data; - data->delay_time = USEC; rcode = cf_section_parse(cs, data, detail_config); if (rcode < 0) { @@ -791,8 +813,8 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this) return -1; } - if ((data->poll_interval < 1) || (data->poll_interval > 3600)) { - cf_log_err(cf_sectiontoitem(cs), "poll_interval must be between 1 and 3600"); + if ((data->poll_interval < 1) || (data->poll_interval > 20)) { + cf_log_err(cf_sectiontoitem(cs), "poll_interval must be between 1 and 20"); return -1; } @@ -827,6 +849,8 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this) data->vps = NULL; data->fp = NULL; data->state = STATE_UNOPENED; + data->delay_time = data->poll_interval * USEC; + data->signal = 1; /* * Initialize the fake client. @@ -842,12 +866,4 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this) return 0; } - -int detail_poll_interval(rad_listen_t *listener) -{ - listen_detail_t *data = listener->data; - - return data->poll_interval; -} - #endif diff --git a/src/main/event.c b/src/main/event.c index c455f06..3134ae4 100644 --- a/src/main/event.c +++ b/src/main/event.c @@ -3038,6 +3038,8 @@ REQUEST *received_proxy_response(RADIUS_PACKET *packet) void event_new_fd(rad_listen_t *this) { char buffer[1024]; + + if (this->status == RAD_LISTEN_STATUS_KNOWN) return; this->print(this, buffer, sizeof(buffer)); @@ -3115,31 +3117,18 @@ static void handle_signal_self(int flag) for (this = mainconfig.listen; this != NULL; this = this->next) { - int delay; - struct timeval when; - if (this->type != RAD_LISTEN_DETAIL) continue; - - delay = detail_delay(this); - if (!delay) continue; - fr_event_now(el, &now); - when = now; - tv_add(&when, delay); - - if (delay > (USEC / 10)) { - DEBUG("Delaying next detail event for %d.%01u seconds.", - delay / USEC, (delay % USEC) / 100000); - } + /* + * This one didn't send the signal, skip + * it. + */ + if (!this->decode(this, NULL)) continue; /* - * Reset the detail timer. + * Go service the interrupt. */ - if (!fr_event_insert(el, event_poll_detail, this, - &when, NULL)) { - radlog(L_ERR, "Failed remembering detail timer"); - exit(1); - } + event_poll_detail(this); } } #endif @@ -3150,8 +3139,6 @@ static void handle_signal_self(int flag) for (this = mainconfig.listen; this != NULL; this = this->next) { - if (this->status == RAD_LISTEN_STATUS_KNOWN) continue; - event_new_fd(this); } } @@ -3232,6 +3219,9 @@ static void event_socket_handler(fr_event_list_t *xel, UNUSED int fd, } } +typedef struct listen_detail_t { + fr_event_t *ev; +} listen_detail_t; /* * This function is called periodically to see if this detail @@ -3239,25 +3229,16 @@ static void event_socket_handler(fr_event_list_t *xel, UNUSED int fd, */ static void event_poll_detail(void *ctx) { - int rcode; + int rcode, delay; RAD_REQUEST_FUNP fun; REQUEST *request; rad_listen_t *this = ctx; struct timeval when; - - fr_event_now(el, &now); - when = now; + listen_detail_t *detail = this->data; rad_assert(this->type == RAD_LISTEN_DETAIL); /* - * Set the next poll time to be X +/- 0.5s, to help - * spread the load a bit over time. - */ - when.tv_sec += detail_poll_interval(this); - when.tv_usec += fr_rand() % (USEC / 2); - - /* * Try to read something. * * FIXME: This does poll AND receive. @@ -3272,15 +3253,17 @@ static void event_poll_detail(void *ctx) } } + if (!fr_event_now(el, &now)) gettimeofday(&now, NULL); + when = now; + /* - * Reset the poll to fire one second from now. If the - * detail reader DOES read a packet, it will send us a - * signal when it's done, and the signal handler will - * reset the timer to a more appropriate (i.e. shorter) - * value. + * Backdoor API to get the delay until the next poll time. */ + delay = this->encode(this, NULL); + tv_add(&when, delay); + if (!fr_event_insert(el, event_poll_detail, this, - &when, NULL)) { + &when, &detail->ev)) { radlog(L_ERR, "Failed creating handler"); exit(1); } @@ -3471,23 +3454,19 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag) switch (this->type) { #ifdef WITH_DETAIL - struct timeval when; - case RAD_LISTEN_DETAIL: DEBUG("Listening on %s", buffer); /* - * Add some initial jitter to help spread - * the load a bit. + * Detail files are always known, and aren't + * put into the socket event loop. */ - when.tv_sec = fr_start_time + 1; - when.tv_usec = fr_rand() % USEC; + this->status = RAD_LISTEN_STATUS_KNOWN; - if (!fr_event_insert(el, event_poll_detail, this, - &when, NULL)) { - radlog(L_ERR, "Failed creating detail poll timer"); - exit(1); - } + /* + * Set up the first poll interval. + */ + event_poll_detail(this); break; #endif @@ -3509,15 +3488,6 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag) break; } - /* - * The file descriptor isn't ready. Poll for - * when it will become ready. This is for the - * detail file fd's. - */ - if (this->fd < 0) { - continue; - } - event_new_fd(this); } -- 2.1.4