[asterisk-dev] [Code Review] New application JabberReceive, implement SendText in chan_gtalk and chan_jingle

[John Todd]

On Dec 12, 2008, at 11:07 AM, Russell Bryant wrote:
>> On 2008-12-11 22:02:02, Russell Bryant wrote:
>>> http://svn.digium.com/svn/asterisk/trunk/res/res_jabber.c, lines  
>>> 821-822
>>> <http://reviewboard.digium.com/r/88/diff/1/?file=1872#file1872line821 
>>> >
>>>
>>>    This is an interesting way to implement this feature.  I  
>>> especially like that it is minimally invasive.  However, I do see  
>>> some downsides to the way this currently works.
>>>
>>>    The first problem I see is that there is technically a race  
>>> condition related to receiving messages.  In the examples, you  
>>> send a message, and then you run an application to receive a  
>>> message.  Unfortunately, there is a period of time in between  
>>> those two where a message might come in, and if your channel isn't  
>>> ready to receive it, it will get lost.
>>>
>>>    The way that I envisioned doing this is to have an option that  
>>> enabled receiving messages globally.  Then, when messages are  
>>> received, you could cache them for some configurable period of  
>>> time.  You could also have a configurable cache size limit.   
>>> Finally, the receive application could do a couple of things:
>>>
>>>    1) Check to see if the message you want to receive has already  
>>> come in and is in the cache.  If so, remove it and return it.
>>>
>>>    2) Add yourself to a list of channels waiting for some  
>>> configurable amount of time for a message to come in.  When  
>>> messages come in, before caching them, they get offered to callers  
>>> waiting for messages.
>>>
>>>
>>>    Those are just some thoughts I had about this functionality.   
>>> The downside is that the code would get a lot more complicated.   
>>> The benefit is that it handles the possibility of a message  
>>> arriving before the dialplan has reached the point of being ready  
>>> to handle it.
>>
>> wrote:
>>    Maybe we can check the call to ast_queue_frame to ensure the  
>> incoming message will be delivered to a listening channel?
>>
>>    All incoming messages that come to Asterisk on its XMPP  
>> client(s) are handled in aji_handle_message. We could acknowledge  
>> the message if we actually queue it as a text frame (when calling  
>> ast_queue_frame, as the return codes don't provide much more  
>> information), for example by sending a notification that Asterisk  
>> 'is composing' a reply. Or we could inform the user that his  
>> message won't be processed and possibly ask him to send it again.
>>
>>    The cache mechanism you describe prevents us from losing  
>> messages, which is more powerful than just informing the user about  
>> the message delivery process. On the other hand, Asterisk frames  
>> along with the useful queuing functionality makes the latter option  
>> possibly acceptable (apart from the fact that it will save me some  
>> work too :)). What's your opinion?
>>
>>    Russell, thanks a lot for taking the time to review this code,  
>> and for your feedback as well.
>>
>> wrote:
>>    What if it's not a user, but an automated system?  Asking the  
>> other side to resend the message is not very ideal.
>>
>>    Another benefit of this other route is that it avoids the  
>> channel_walk_locked() loop, which is very expensive.  An  
>> optimization would be the new ast_channel_search() API call.   
>> However, this other route avoids the need for searching the channel  
>> list at all, and will make a big difference on heavily loaded  
>> systems.
>>
>>    I would be happy to help with this new approach if you would  
>> like to discuss implementation specifics in more detail on the -dev  
>> list.
>>
>> wrote:
>>    Ok Russell, I understand. With the current approach, we can't  
>> avoid calling channel_walk_locked (or its optimized version  
>> ast_channel_search). Storing the messages in Asterisk's messages  
>> list, and then going through this list when calling JabberReceive  
>> would indeed avoid searching the channel list.
>>
>>    I can come up with an updated version that also integrates John  
>> Todd's suggestion. Do you agree with leaving the current approach  
>> (text frames queued to listenign channels), and only search the  
>> message list in Asterisk's XMPP client(s) when calling JabberReceive?
>
> To be honest, no, I do agree with leaving the current approach.  I  
> consider the chance for lost messages a flaw that we can't really  
> ignore.  Feel free to ask for other opinions on the -dev list,  
> though, if you'd like.
>
>
> - Russell

After some thought, I'd have to agree with Russell on this as well -  
there needs to be some way to capture messages that would be sent  
prior to the JabberRecieve being called.

[note: the next two paragraphs are the start of what will become a  
very, very long set of religious discussions and gnashing of teeth and  
tearing of hair]

Why?  Because I suspect that this will almost immediately become  
another API method for Asterisk.  It's an ugly, horrible, nasty  
hackish API but guess what - it'll work for most people doing  
lightweight interactions who are already using Jabber.  Machine-to- 
Machine interactions via XMPP is becoming common, and therefore the  
responses back from JabberSend will be lightning fast in many  
instances, and even the time it takes for a few dialplan events may be  
too long to correctly receive an inbound reply from an outbound  
message.  At the very least, the implementation that gets shipped  
first should be able to handle immediate responses to transmitted  
messages that may be generated by other systems otherwise the first  
impressions will be the last impressions someone gets on using this  
method, and it will create bogus summaries and manual pages that will  
be difficult or impossible to correct later - "You can't use the  
JabberSend/JabberReceive apps in Asterisk for fast responses because  
they will lose messages." would be an example of what I do NOT want to  
see.  There will certainly be problems with M2M systems using XMPP in  
this way, but such a significant one I think we should avoid on our  
first step into the minefield.

Of course, an API should have the ability to create actions in a  
system, but the elements Russell mentions (global receipt of XMPP  
messages) makes that a fairly simple jump.  My opinion is that XMPP  
messages should be received globally on any JIDs registered in  
jabber.conf, and then should execute in the dialplan on a per- 
registration context (just like any other channel) using the  
registered JID as the $EXTEN, using the remote JID as the caller ID,  
and setting the text value as a dialplan variable.   But... one step  
at a time, I suppose.  :-)

Philippe - I understand that it works now the way you have it, but  
would it delay implementation significantly?  Are you on a roll, and  
could you keep that momentum going to add the pre-buffering or global  
receipt that has been discussed above?  I think there is an  
opportunity to have a big leap in functionality of Asterisk, and it's  
your code that's at the heart of that advancement.

JT

---
John Todd
jtodd at digium.com        +1-256-428-6083
Asterisk Open Source Community Director