question about concurrent auth packet process.
When lots of concurrent auth packets arrived at radius server .by default ,if radius server exceeds the max packet process thread ,it will discard the auth packet/ see the code below in version 1.0.5 int thread_pool_addrequest(REQUEST *request, RAD_REQUEST_FUNP fun) { /* * If the thread pool is busy handling requests, then * try to spawn another one. */ if (thread_pool.active_threads == thread_pool.total_threads) { if (spawn_thread(request->timestamp) == NULL) { radlog(L_INFO, "The maximum number of threads (%d) are active, cannot spawn new thread to handle request", thread_pool.max_threads); return 0; } } /* * Add the new request to the queue. */ request_enqueue(request, fun); return 1; } why not just add the auth packet to the queue of the thread if the active thread number exceed the total thread number? -- Yesterday is a history. Tomorrow is a mystery. Today is a gift. That's why we call it "the Present".
On December 29, 2005 3:45:12 PM +0800 Liu dejun <dejun.liu@gmail.com> wrote:
When lots of concurrent auth packets arrived at radius server .by default ,if radius server exceeds the max packet process thread ,it will discard the auth packet/ ... why not just add the auth packet to the queue of the thread if the active thread number exceed the total thread number?
good point. -frank
if this method is ok,but the default queue size is 65536 pls check the code below in function request_enqueue() : if (thread_pool.queue_size >= 65536) { pthread_mutex_unlock(&thread_pool.mutex); /* * Mark the request as done. */ radlog(L_ERR|L_CONS, "!!! ERROR !!! The server is blocked: discarding new request %d", request->number); request->finished = TRUE; return; } The queue size is too big !! if thread's queus is full of auth packet or acct packet , the thread is busy processing the packets,the packet will time out by the thread clean process,so 65536 is too big for thread queue size. 2005/12/29, Frank Cusack <fcusack@fcusack.com>:
On December 29, 2005 3:45:12 PM +0800 Liu dejun <dejun.liu@gmail.com> wrote:
When lots of concurrent auth packets arrived at radius server .by default ,if radius server exceeds the max packet process thread ,it will discard the auth packet/ ... why not just add the auth packet to the queue of the thread if the active thread number exceed the total thread number?
good point.
-frank - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
-- Yesterday is a history. Tomorrow is a mystery. Today is a gift. That's why we call it "the Present".
Liu dejun <dejun.liu@gmail.com> wrote:
The queue size is too big !! if thread's queus is full of auth packet or acct packet , the thread is busy processing the packets,the packet will time out by the thread clean process,so 65536 is too big for thread queue size.
No. "too big" is a function of how fast the machine is, what kind of requests come in, and how many requests the machine can handle. If a machine can handle 5000 requests/second, then the queue contains only 13 seconds of request processing, so the requests won't time out. e.g. If a machine normally handles 1K requests/s, but is capable of handling 5k requests/s, then a sudden "spike" of 10k requests/s will cause the queue to fill up. So long as the spike is smaller than 10 seconds or so, there shouldn't be a problem. In that case, it could be argued that a queue of 64k requests is too small. I have some trial patches in my sandbox that expire requests from the queue if they've been there too long. This allows the server to recover from huge spikes. I have some other trial patches that create 3 queues: One for new requests, one for proxy replies from home servers, and one for ongoing EAP sessions. The queues have different levels of priority, and it's preferable to drop new requests in order to finish processing previous requests. Alan DeKok.
On December 29, 2005 12:57:34 PM -0500 Alan DeKok <aland@ox.org> wrote:
Liu dejun <dejun.liu@gmail.com> wrote:
The queue size is too big !! if thread's queus is full of auth packet or acct packet , the thread is busy processing the packets,the packet will time out by the thread clean process,so 65536 is too big for thread queue size.
No.
"too big" is a function of how fast the machine is, what kind of requests come in, and how many requests the machine can handle. If a machine can handle 5000 requests/second, then the queue contains only 13 seconds of request processing, so the requests won't time out.
What is the typical timeout for a NAS device? I am pretty sure that pam_radius' default timeout is only 3s. -frank
Frank Cusack <fcusack@fcusack.com> wrote:
What is the typical timeout for a NAS device? I am pretty sure that pam_radius' default timeout is only 3s.
Most NASes it's 15-30 seconds. 3-5 seconds between retransmits, and 3-5 retransmits. Any retransmits are caught by the server core, and discarded. They don't go into the queue. Alan DeKok.
In my switch,the request timeout values is 3s,and 3 retrasmits On 12/30/05, Alan DeKok <aland@ox.org> wrote:
Frank Cusack <fcusack@fcusack.com> wrote:
What is the typical timeout for a NAS device? I am pretty sure that pam_radius' default timeout is only 3s.
Most NASes it's 15-30 seconds. 3-5 seconds between retransmits, and 3-5 retransmits.
Any retransmits are caught by the server core, and discarded. They don't go into the queue.
Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
-- Yesterday is a history. Tomorrow is a mystery. Today is a gift. That's why we call it "the Present".
Liu dejun <dejun.liu@gmail.com> wrote:
When lots of concurrent auth packets arrived at radius server .by default ,if radius server exceeds the max packet process thread ,it will discard the auth packet/ see the code below in version 1.0.5
This is fixed in 1.1.0. See the 1.1.0-pre0 source for details. Alan DeKok.
In threads.c ,I find the code below int thread_pool_addrequest(REQUEST *request, RAD_REQUEST_FUNP fun) { /* * Add the new request to the queue. */ if (!request_enqueue(request, fun)) return 0; /* * If the thread pool is busy handling requests, then * try to spawn another one. */ if (thread_pool.active_threads == thread_pool.total_threads) { if (spawn_thread(request->timestamp) == NULL) { radlog(L_INFO, "The maximum number of threads (%d) are active, cannot spawn new thread to handle request", thread_pool.max_threads); return 1; } } return 1; } why not restricted the conditions ,and add the if condition to just if the total threads exceed the max threads,if so just add to the thread queue and return ,if not do the spawn thread function to spawn a new thread .and do this ,it will reduce many functioin calls is this function,at the same time it will reduce too much time when too many packets arrived. is my idea right? and also in file threads.c,the if condition if (thread_pool.total_threads >= thread_pool.max_threads) { DEBUG2("Thread spawn failed. Maximum number of threads (%d) already running.", thread_pool.max_threads); return NULL; } is useless. tks in advanced? On 12/30/05, Alan DeKok <aland@ox.org> wrote:
Liu dejun <dejun.liu@gmail.com> wrote:
When lots of concurrent auth packets arrived at radius server .by default ,if radius server exceeds the max packet process thread ,it will discard the auth packet/ see the code below in version 1.0.5
This is fixed in 1.1.0. See the 1.1.0-pre0 source for details.
Alan DeKok.
- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
-- Yesterday is a history. Tomorrow is a mystery. Today is a gift. That's why we call it "the Present".
Liu dejun <dejun.liu@gmail.com> wrote:
why not restricted the conditions ,and add the if condition to just if the total threads exceed the max threads,if so just add to the thread queue and return ,if not do the spawn thread function to spawn a new thread .and do this ,it will reduce many functioin calls is this function,at the same time it will reduce too much time when too many packets arrived.
I'm not sure I understand what you mean by this. Could you send a patch implementing what you suggest? I *think* you mean to always spawn a new thread, until "max_threads" is hit. If that is what you mean, then the current design is intended to *avoid* spawning new threads as much as possible. The overhead of spawning a new thread is much, much, more than a function call. So the server does not spawn a new thread unless it absolutely has to. Alan DeKok.
participants (3)
-
Alan DeKok -
Frank Cusack -
Liu dejun