[asterisk-dev] [Code Review] 4572: RFC: Refactor Qualify and res_pjsip/endpt_send_request

[George Joseph]

> On April 1, 2015, 6:21 p.m., Mark Michelson wrote:
> > As a concept, this feels like a suitable stop-gap solution. Better solutions would be alterations to PJSIP itself. Either
> > 
> > 1) Make pjsip_endpt_send_request() not ignore the timeout parameter passed to it.
> > 2) Make pjsip transaction timers configurable per transaction.
> > 
> > The solution provided here has the issue that it has an Asterisk scheduler and a PJLIB timer competing with each other, when it would be less wasteful to have the PJLIB timer do everything. Because of the competing timers, it's possible for the transaction state callback to be called into from multiple threads at the same time. If the transaction timers could be altered per transaction, then there would only ever be a single timer running, and the possibility of races is eliminated.

I'm going to work on a pjproject patch as well but it'll probably take a while to get mainlined.  The sched timer itself doesn't run the callback when it triggers, it only cancels the transaction and pjsip runs the callback so the callback should never be called by multiple threads.


- George


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


On March 30, 2015, 11:57 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4572/
> -----------------------------------------------------------
> 
> (Updated March 30, 2015, 11:57 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24863
>     https://issues.asterisk.org/jira/browse/ASTERISK-24863
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This review is based on the discussion here
> http://lists.digium.com/pipermail/asterisk-dev/2015-March/073921.html
> 
> -------------------------------------
> Right now when a contact is qualified, it's immediately marked as
> UNAVAILABLE.  If a response to the OPTIONS message comes back, it's
> marked as AVAILABLE again and the round trip time calculated.   The
> status flapping even in normal operation makes generating events
> tricky because the subscriber will see up/down events even if the
> contact is really AVAILABLE.   Now the pjsip transaction will
> eventually time out at an unconfigurable 32 seconds and call the
> callback but 32 seconds is a long time.  This is also an issue if the
> qualify_frequency is less than 32 seconds since they'll be multiple
> pjsip transactions in progress for the same contact.
> 
> <snip>
> 
> So, I'm proposing pulling libpjsip/pjsip_endpt_send_request up into
> res_pjsip.   It's 2 short function and a pjsip_module structure with
> no access to any private pjsip stuff.   Now we'll have the ability to
> terminate the transaction AND it seems that there's a timeout member
> of pjsip_transaction which I'm hoping (but haven't tested) will
> eliminate the need to add timeout processing in pjsip_options.
> 
> The result of all of this is that we'll be able to generate events for
> contact status (ASTERISK-24863) which in turn will help is provide
> functionality like marking an endpoint unavailable when all of its
> contacts are unavailable.
> -------------------------------------
> 
> Both Josh and Mark had comments in the thread so this is an RFC ONLY review.
> It covers the pull up of libpjsip/pjsip_endpt_send_request but I've also added the
> use case which is creating and processing a new aor/contact option called 
> 'qualify_timeout'.  I'll split the final submission into 2 pieces.
> 
> 
> Diffs
> -----
> 
>   branches/13/res/res_pjsip/pjsip_options.c 433794 
>   branches/13/res/res_pjsip/location.c 433794 
>   branches/13/res/res_pjsip.c 433794 
>   branches/13/include/asterisk/res_pjsip.h 433794 
> 
> Diff: https://reviewboard.asterisk.org/r/4572/diff/
> 
> 
> Testing
> -------
> 
> All existing pjsip related tests complete.  I'll also have to add event generation on contact status change and the tests based on it.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150402/f51ad40e/attachment.html>