question about concurrent auth packet process.

classic Classic list List threaded Threaded
10 messages Options
| Threaded
Open this post in threaded view
|

question about concurrent auth packet process.

Liu dejun
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".
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Frank Cusack
On December 29, 2005 3:45:12 PM +0800 Liu dejun <[hidden email]> 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
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Liu dejun
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 <[hidden email]>:
On December 29, 2005 3:45:12 PM +0800 Liu dejun <[hidden email]> 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".
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Alan DeKok
In reply to this post by Liu dejun
Liu dejun <[hidden email]> 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
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Alan DeKok
In reply to this post by Liu dejun
Liu dejun <[hidden email]> 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.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Frank Cusack
On December 29, 2005 12:57:34 PM -0500 Alan DeKok <[hidden email]> wrote:

> Liu dejun <[hidden email]> 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
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Alan DeKok
Frank Cusack <[hidden email]> 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
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Liu dejun
In my switch,the request timeout values is 3s,and 3 retrasmits

On 12/30/05, Alan DeKok <[hidden email]> wrote:
Frank Cusack <[hidden email]> 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".
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Liu dejun
In reply to this post by 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 <[hidden email]> wrote:
Liu dejun <[hidden email]> 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".
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: question about concurrent auth packet process.

Alan DeKok
Liu dejun <[hidden email]> 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.

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html