[asterisk-dev] [Code Review] Append two container for dialogs to delete and for rtp timeout checks to replace all dialogs container in ao2 callback for dialogs_needdestroy

[David Vossel]

> On 2010-09-13 12:13:47, David Vossel wrote:
> > branches/1.6.2/channels/chan_sip.c, lines 22760-22763
> > <https://reviewboard.asterisk.org/r/917/diff/2/?file=12491#file12491line22760>
> >
> >     What do we gain by running this sooner than 1000ms?  If a Request or Response comes in, or some other event that requires the do_monitor loop it is supposed to break out early from this wait.
> 
> schmidts wrote:
>     the problem is if the ast_sched_wait returns something > 1000 the ast_io_wait function waits for 1 sec or if a package is received. if an event is scheduled earlier than this 1 sec it only would start after ast_io_wait has finished. i dont think this would happens offen but its possible. so reducing the max time which ast_io_wait will poll to 100ms could speed up a short scheduled event.
> 
> David Vossel wrote:
>     Ah, yes.  I understand where you are coming from now.  You shouldn't have to worry about this. The ast_sched_wait() function call the line right before this determines when the next scheduled event will occur.  If it is less than 1000ms, then the wait only happens until the next event.
> 
> schmidts wrote:
>     i know this ;) but maybe i was on the wrong path. is there a case where something could place an event without an incoming request or response will come in? i am not sure if this ever could happen so there is really no need to turn down the waiting time.
>     i changed this back.
>

The only reason I can think of to increase the wait time is for an event that gets scheduled outside of the do_monitor thread that is less that 1000ms.  I have run into this on one occasion. In __sip_reliable_xmit we transmit a sip Request/Response and set the retransmit scheduler event to occur in 500ms.  To account for this I did a 'pthread_kill(monitor_thread, SIGURGE)' call on the monitor thread to make sure it woke up to processes the new event correctly.  Otherwise if the transmit happened outside of the monitor thread it was possible the retransmit would not occur at the right time.

I'm not necessarily opposed to shortening this time, but since we are on the route of trying maximize performance, I just want to make sure we have a good reason to do it first.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/917/#review2714
-----------------------------------------------------------


On 2010-09-11 17:44:40, schmidts wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/917/
> -----------------------------------------------------------
> 
> (Updated 2010-09-11 17:44:40)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Instead of iterate through all sip dialogs in the do_monitor function of chan_sip.c i created two separate containers which links to dialogs. 
> The container dialogs_needdestroy links to dialogs which are marked as needdestroy, so they should be unrefered from the dialogs container.
> The container dialogs_rtpcheck links to dialogs which have active rtp, vrtp or trtp and rtptimeout, rtpholdtimeout or rtpkeepalive activated.
> 
> normaly the container with needdestroy dialogs is empty so the callback takes only around 30 usec. even if there are more than 10 dialogs to be destroyed it takes around 200 usec. 
> the callback for the rtpcheck tooks around 200 usec with 30 concurrent calls and 700 usec if the timeout is reached to do a softhangup.
> 
> further i have found in check_rtp_timeout a possible locking situation. i am not sure if this would be a working solution cause it could happen that a call would be hung up several ms or also 1 sec later.
> 
> i think after speeding the ao2 callback up, its not necessary to wait 1000 ms, with 100ms dialogs would be faster released, the timeout fires even shorter and scheduled events which has been scheduled in a very short future time would be fired faster. see change 40 for this.
> 
> i am also not sure if its necessary to iterate through the containers on module_unload to unlink the entries. the dialogs itself would be destroyed after they are unrefered from the dialogs container. see change 42 for this.
>  
> 
> 
> This addresses bug 17912.
>     https://issues.asterisk.org/view.php?id=17912
> 
> 
> Diffs
> -----
> 
>   branches/1.6.2/channels/chan_sip.c 286373 
> 
> Diff: https://reviewboard.asterisk.org/r/917/diff
> 
> 
> Testing
> -------
> 
> i have tested this with sipp and 500 concurrent calls all with rtptimeout 10 s and the active time was 10,9s per call.
> also the 20k peers register test run through without any problem. (with the astdbpatch)
> 
>                                  Messages  Retrans   Timeout   Unexpected-Msg
>       INVITE ---------->         500       0
>          100 <----------         500       0                   0
>          200 <----------         500       0                   0
>          ACK ---------->         500       0
>        Pause [    40.0s]         500                           500
>          BYE ---------->         0         0         0
>          200 <----------         0         0                   0
> ------------------------------ Test Terminated --------------------------------
> 
> 
> ----------------------------- Statistics Screen ------- [1-9]: Change Screen --
>   Start Time             | 2010-09-11 23:12:32
>   Last Reset Time        | 2010-09-11 23:12:47
>   Current Time           | 2010-09-11 23:12:48
> -------------------------+---------------------------+--------------------------
>   Counter Name           | Periodic value            | Cumulative value
> -------------------------+---------------------------+--------------------------
>   Elapsed Time           | 00:00:00:964              | 00:00:15:994
>   Call Rate              |    0.000 cps              |   31.262 cps
> -------------------------+---------------------------+--------------------------
>   Incoming call created  |        0                  |        0
>   OutGoing call created  |        0                  |      500
>   Total Call created     |                           |      500
>   Current Call           |        0                  |
> -------------------------+---------------------------+--------------------------
>   Successful call        |        0                  |        0
>   Failed call            |       35                  |      500
> -------------------------+---------------------------+--------------------------
>   Call Length            | 00:00:10:970              | 00:00:10:637
> ------------------------------ Test Terminated --------------------------------
> 
> 
> Thanks,
> 
> schmidts
> 
>