Fixed detail file handler to not go crazy
authorAlan T. DeKok <aland@freeradius.org>
Sat, 11 Apr 2009 07:32:39 +0000 (09:32 +0200)
committerAlan T. DeKok <aland@freeradius.org>
Sat, 11 Apr 2009 07:32:39 +0000 (09:32 +0200)
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
src/main/detail.c
src/main/event.c

index 0ee4cb1..699b1be 100644 (file)
@@ -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 */
index f582817..3bb97bd 100644 (file)
@@ -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
index c455f06..3134ae4 100644 (file)
@@ -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);
        }