On Tue, Oct 20, 2009 at 12:13:26PM +0200, Alan DeKok wrote:
John Morrissey wrote:
Recently, one of the home servers in a load balanced pool was configured with the wrong secret, triggering this code in received_proxy_response(): ... Since the proxy response is ignored and detail_send() is never called, the detail listener stays in STATE_RUNNING. detail_recv() short-circuits during STATE_RUNNING, so the failed request is never retransmitted and all detail processing for that listener halts until FreeRADIUS is restarted.
OK. That should be fixed.
The solution is to fix detail_recv() so that it retries the packet after being in the RUNNING state for "retry_interval".
I'll commit a patch.
Thanks, Alan. FWIW, I reported this against 2.1.x but backported the fix to 2.0.x (patch below, for the list archives' sake), since we're trying to stay close to the Debian packaging. So far, we've managed to move a fairly large installation from Radiator (I'm guesstimating 600 incoming auth+acct requests/sec) with only this patch, so we decided it wasn't worth it to diverge from stock at this point. john --- freeradius/src/main/event.c.orig 2009-10-20 18:26:20.020171000 +0000 +++ freeradius/src/main/event.c 2009-10-20 18:27:41.744485000 +0000 @@ -2392,7 +2392,7 @@ when.tv_sec += 1; for (this = mainconfig.listen; this != NULL; this = this->next) { - if (this->fd >= 0) continue; + if (this->fd >= 0 && this->type != RAD_LISTEN_DETAIL) continue; /* * Try to read something. --- freeradius/src/main/detail.c.orig 2008-05-09 12:32:24.000000000 +0000 +++ freeradius/src/main/detail.c 2009-10-20 16:34:29.381519000 +0000 @@ -49,9 +49,11 @@ FILE *fp; int state; time_t timestamp; + time_t running; fr_ipaddr_t client_ip; int load_factor; /* 1..100 */ int signal; + int retry_interval; int has_rtt; int srtt; @@ -429,11 +431,18 @@ goto alloc_packet; /* - * We still have an outstanding packet. - * Don't read any more. + * Periodically check what's going on. + * If the request is taking too long, + * retry it. */ case STATE_RUNNING: - return 0; + if (time(NULL) < (data->running + data->retry_interval)) { + return 0; + } + + DEBUG("No response to detail request. Retrying"); + data->state = STATE_NO_REPLY; + /* FALL-THROUGH */ /* * If there's no reply, keep @@ -691,6 +700,7 @@ } data->state = STATE_RUNNING; + data->running = packet->timestamp; return 1; } @@ -744,6 +754,8 @@ offsetof(listen_detail_t, filename), NULL, NULL }, { "load_factor", PW_TYPE_INTEGER, offsetof(listen_detail_t, load_factor), NULL, Stringify(10)}, + { "retry_interval", PW_TYPE_INTEGER, + offsetof(listen_detail_t, retry_interval), NULL, Stringify(30)}, { NULL, -1, 0, NULL, NULL } /* end the list */ }; -- John Morrissey _o /\ ---- __o jwm@horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__