[Home]

Summary:ASTERISK-02926: MWI system is currently polling based, this makes it event based and fixed ODBC storage issues
Reporter:Michael Shuler (mikes2277)Labels:
Date Opened:2004-12-05 12:08:33.000-0600Date Closed:2011-06-07 14:10:33
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) MWI_And_Ast_Data_Version_10.tar.gz
( 1) MWI_And_Ast_Data_Version_11.tar.gz
( 2) MWI_Version_10.tar.gz
( 3) MWI_Version_11.tar.gz
Description:Since the new ODBCstorage voicemail system uses res_odbc but the message counter function is in app.c it can't make use of res_odbc without breaking the modularity of Asterisk.  This set of patches fixes that issue and gets rid of the unnescessary polling on do_monitor() (chan_sip, chan_iax2, etc.) of every peer's mailbox to see if there are new messages.  Instead a MWI queue is created which do_monitor() only has to check once for any new entries.  This significantly cuts down on CPU and disk usage times the number of users in the system.
Comments:By: Brian West (bkw918) 2004-12-05 13:15:25.000-0600

patch.. ? :P

bkw

By: Michael Shuler (mikes2277) 2004-12-05 13:48:15.000-0600

find a memory leak last second, so I fixed it first :)

By: Michael Shuler (mikes2277) 2004-12-05 13:50:07.000-0600

I'm finishing up the message counter replacement functions so they don't have to be part of app.c so be expecting another patch later this week.  Let me know what you think so far :)

By: Kevin P. Fleming (kpfleming) 2004-12-05 15:55:24.000-0600

OK, maybe I'm missing something blindingly obvious, but how is this supposed to work when Asterisk is restarted? What about if the voicemail spool is shared among multiple servers (recent patches to support ODBC storage of voicemail seem to be moving in that direction)?

I think at a minimum you need to put in a "startup" task to scan the voicemail spool and populate the MWI task list with all mailboxes that have messages in their INBOXes at that time; that would at least take care of the restart problem.

The shared spool problems is much, much harder to solve: it will likely require putting the MWI task list into that same shared storage, so that all participants can use it and keep it up to date.

By: Michael Shuler (mikes2277) 2004-12-06 21:53:48.000-0600

Actaully I wrote this for my purposes which runs the ast_data patch (which is actually realtime unlike the "realtime" stuff in Asterisk now).  In my situation Asterisk (which is actually an Asterisk cluster thanks to ast_data) never shuts down.  So the startup issue isn't really a concern of mine since my "virtual" Asterisk server never goes offline, even if it did the VoIP devices would still retain their last MWI status.  Also, sending to ALL VoIP devices on startup in my situation would cause 10K+ NOTIFY messages to go out of each server in my cluster all at the same time... that would not be good :)Ahh, but you did make me realize that if the phone loses power that it also loses its MWI status.  Soooo, how about if I make it send a MWI status if the REGISTER c-seq is 1?  I don't think I have ever seen a VoIP device that doesn't start on 1 when its first turned on.  However in my situation what wouldn't work because SER sees the REGISTER messages not Asterisk.... Hmmmm.  Perhapse a external program in SER that checks the MySQL VM DB for any messages for that users mailbox and then generates a NOTIFY within SER.  Guess that would solve both problems.  Any thoughts?

By: Michael Shuler (mikes2277) 2004-12-06 21:59:05.000-0600

After looking at the message counter stuff it seems really silly that they are in app.c if my MWI patch is in use.  After all the point of Asterisk is to be modular and putting voicemail specific stuff in app.c seems very defeating of this concept.  So, I propose all voicemail related stuff get removed completely from app.c... all that seems to be there is stuff to count voicemail messages for sending MWI anyway which my patch gets rid of the need to since counting would actaully be done inside the app_voicemail module, which is where it should have been to begin with.  Any thoughts before I implement this?

By: Kevin P. Fleming (kpfleming) 2004-12-06 22:10:41.000-0600

Yes, the "power off" issue is definitely what I was referring to regarding the VOIP devices. If you only send MWI once, it can be lost.

However, based on my cursory reading of your patch, you continue sending MWI according the current procedures, you just optimize the searching for mailboxes that have messages waiting (by using the task list, rather than scanning the mailboxes). If that's the case, then even with your patch the recently-booted devices will still get MWI turned on, it will just not happen until their next NOTIFY cycle occurs. For users who are using Asterisk's "qualify" feature with a reasonably short time period, this occurs frequently, so the MWI will get turned back on in short order.

How does your "cluster" handle MWI across nodes? If node A puts a message into phone 123's mailbox, but phone 123 is currently being sent NOTIFY messages by node B, how is node B going to become aware that MWI should be turned on?

I do agree that all voicemail-related functions should be moved into app_voicemail; all that should be left is some method for channel technology drivers to query the voicemail module for mailboxes with unread messages that they may be interested in. The "task list" in your patch seems to suit that need very well :-)

I would definitely add a startup task to get it populated on Asterisk restarts though; most of us do not have a "never down" Asterisk cluster like you do. I'm not suggesting that you send NOTIFY to every device on startup; I'm suggesting that you scan all known mailboxes _one time_ to build the task list as it would have been when Asterisk was last running, so that the next time the channel drivers want to find out if a peer has messages waiting, the task list will accurately reflect the state of the mailboxes. In other words, a complete mailbox scan is still needed, but only once at startup, to sychronize the in-memory task list with the state of the mailboxes on disk (or in the database, or wherever).

By: Olle Johansson (oej) 2004-12-07 01:52:53.000-0600

Is this code disclaimed?

By: Olle Johansson (oej) 2004-12-07 01:58:37.000-0600

-#CFLAGS+=-DUSE_ODBC_STORAGE
+CFLAGS+=-DUSE_ODBC_STORAGE

-- Do not change default behaviour in the code
-- Read coding guidelines! // comments should be replaced by /* */

By: khb (khb) 2004-12-08 13:02:40.000-0600

I am not sure this method really solves much.  If you have a large voicemail system and a cluster of servers, then the current vm status notification system is completely wrong approach and I wouldn't try to fix it.
The voicemail notification system needs to be taken out of the communication technology channels (SIP, IAX, MGCP...).  There should only be a single separate application or module that maintains the state of the mail boxes and sends notifications according to whatever algorithm is selected via a service function in the desired technology channel. If a user is logged in with, say, IAX and SIP at the same time then such a system only determines state once and simply calls the IAX and SIP channels to send a notification.  This is the way you create the modularity you were speaking of (which * doesn't really have)  THe way it is now, each technology channel has its own MWI subsystem querying the same mailboxes all the time.

edited on: 12-08-04 13:22

By: Kevin P. Fleming (kpfleming) 2004-12-08 17:07:38.000-0600

khb, I'll certainly agree that the modularity of the existing MWI system is in need of serious rework.

However, this patch does move in the proper direction: it reduces the amount of mailbox scanning, and allows for any other means to put MWI tasks into the list (or remove them) when needed.

The next logical step would be to add a thread/process in app_voicemail that wakes up periodically and scans the task list, calling into the appropriate technology modules to send MWI. The biggest issue there, as you already pointed out, is that currently the voicemail system doesn't have a clue what peers any given mailbox belongs to; the technology drivers would need to somehow communicate that into the voicemail system so it can place the proper callbacks.

By: Andrew Lindh (andrew) 2004-12-09 08:50:20.000-0600

The Polycom phones have a bug/feature that beeps each time they get a MWI update. This is a pain because if the scan rate is set to 10 seconds the Polycom just beeps all the time. It's not smart enough to see there is no change (I disabled the beep in the phones to keep the office sane). Sending MWI on an actual message quantity change, or phone restart, or say once an hour (to make sure a phone did not miss an update) would be a good thing. I know this is not exactly what this patch is for, but it would be a benefit of only updating MWI when a update is actually needed. On an large scale reducing unneeded MWI updates could save a good amount of bandwidth.

By: khb (khb) 2004-12-09 12:07:07.000-0600

kpfleming:    Yes, I agree in principle.

Andrew:   The MWI notification in SIP isn't perfect either. I have updated mine to only send NOTIFY when the mailbox status (new messages) actually changes.

By: Mark Spencer (markster) 2004-12-09 15:06:22.000-0600

There is already an event system which is used for notifying of the status of extensions (see ast_extension_state_add and friends).  How about trying to expand about the existing system to carry this information as well?  There should be both a polled and event driven interface, one to find out what the status is and another to alert to new status.

One method for checking mailbox status would be to store the updated counts of messages in astdb.

By: Michael Shuler (mikes2277) 2004-12-09 18:28:20.000-0600

I finally have time to work on it again tonight, sleep is overrated anyway :)

Well since we now have quite the audience I guess I should really get this done right.  Here is what I propose (keep in mind I have not looked at code from marksters suggestion yet so this may be redundant):

MWI is a PBX type feature and should be handled by all channels.  Since each channel has to implement MWI in its own unique way there needs to be a “ast_sip_send_mwi()” type function for each channel.  Now, to make Asterisk a lean mean module loading machine ™ it should only mess with voice mail and MWI checks if a voice mail module is loaded, such as app_voicemail.  However, since modules can’t see each others functions that also creates a bit of a problem.  It seems to me that there may be other such application that need events to happen to VoIP phones on a timed or event type basis, such as notifying the phone that a holding queue is full or that another user is on the phone (for a switch board/call center monitoring type application).  It would be impossible to predict every situation and hard code it into chan_sip/iax2/mgcp/etc.  However, since each channel must deal with the underlying data sent to the phone it is inevitable that it must be hard coded into the channel.  But then I thought of a possibly better way to do it… What if we work the modules in a backwards sense?  Instead of the voice mail module inserting a MWI event and then waiting for chan_sip’s do_monitor() function to get around to sending it why don’t we have chan_sip “register” its MWI function with a global function pointer list so the voicemail’s “do_monitor” thread can call it?  

The channels (when loaded) would register all of their capabilities into a global “channel capabilities” structure for access by all of the “feature” apps such as voicemail.  The global struct would be based off registered text string names that would be standardized so that the app_xxxx would know what to search for.  For example to send a MWI app_voicemail would search for any MWI functions registered in the global struct and execute the functions for the mailbox that needs notification.  The functions executed are from each channel’s function registration.  If a channel doesn’t support MWI and hence didn’t register a channel, app_voicemail would simply not find a MWI function for it and wouldn’t send a MWI for that channel type.

Another thought is that this would require quite a bit of rewrite and to just leave it the way it is would work fine too.

Any thoughts before I continue tonight on one of the two paths?

By: Kevin P. Fleming (kpfleming) 2004-12-09 18:59:11.000-0600

You are thinking along the right direction, in my opinion, but there are still some flaws.

For example, app_voicemail has no idea which channel drivers or peer names are interested in any given mailbox, so it can't really just call the channel's "update MWI" function from the do_monitor loop.

I think the only logical way for this to work is something like this:

Feature modules can register their _ability_ to provide a given list of features; these could be predefined text strings, managed in a core module that handles the registrations.

Channel modules can register their _desire_ to be notified when a given feature changes state; they do this by calling into the same core module and passing the predefined feature string, along with an opaque blob of data that the feature module can interpret (for voicemail MWI, this would be the "mailbox" string), and a pointer to a structure of function calls that the feature module provides for callbacks. The channel module would also have to pass in an opaque blob for its own use, so that when the callback occurs it can refer to the proper peer/user/etc.

The code module would then scan its list of registered feature modules looking for any that provide the given feature; for each one it finds, it would forward the request into the feature module's control, to see if the feature module wants to do anything with it. If the feature module does, it would then store the pointer to the channel's callback structure, and the channel's "tag" data. When it needs to call one of the channel's functions, it can do so.

There are a lot of issues with a design like this, though (also with yours)... when you start sharing data across modules, you run into lifetime problems. There are also threading problems: if app_voicemail starts up a thread to monitor the MWI task list, and finds something to do, it may place a callback into chan_sip to notify a peer, but chan_sip may not be able to notify the peer at that moment.

In other words, you are right: this is a major design change. I think your current patch does a lot of good and is a much simpler step in the right direction, as long as its known issues can be resolved.

By: Michael Shuler (mikes2277) 2004-12-10 02:49:26.000-0600

OK, bunch of stuff added and fix but still a few things left to be done for antoher day.  Unfortunatly I have meetings early tommorrow so I had to stop short.

Fixed:
 * Global MWI struct wasn't being initalized at startup

 * Send MWI to everyone at startup (option in configs/voicemail.conf)

 * Cycle timer so we don't send startup MWI before the phones have had a chance to REGISTER (option in configs/voicemail.conf)

 * Counting messages is now done in app_voicemail (50% done)



Not Done Yet:
 * Startup only works with plain disk spool and not with ODBC storage *yet*.

 * The disk spool counter is in load_module() and is not a function yet, I will later move it to a function and make a ODBC version too.  These 2 will replace the app.c message counter.

 * Message count is always 1 if there are any messages.  When the counter functions are done this will be fixed.

 * Nothing has been done to deal with shutting off the MWI after all messages have been read or deleted.  This will be done after the counter functions are done, after all we need to know how many messages are left :)  However, *most* phones will automatically turn off their light when you try to check the voice via their voice mail softbuttons anyway (such as a snom).

 * A possible MWI reminder thread in app_voicemail to resend MWI every X seconds.  Will have an option to enable/disable for cluster enviornments to prevent bombardments of MWI.

 * "CSeq: 1 REGISTER" detection for chan_sip.c that will send a MWI if there are messages.  This will solve phones that lost power are have been rebooted.  Not sure how I'm going to do this since chan_sip.c can't call the message counter functions that will eventually be only in app_voicemail.



Other notes:
 * This *should* work with a shared spool.  The idea is that only one Asterisk box will be set to send MWI at startup that way all of them don't send out redundant MWI and bombard the VoIP phones.

 * You may see MWI lights come on before the cycle timer finishes.  This is because the old code is still in there querying all the boxes every cycle.  After the patch is completely done the old code should be removed and the efficiency of a large deplyment will dramatically increase.  If your feeling adventurous feel free to comment out the old code (the most important stuff is around lines 7909-7930 in chan_sip.c)

 * I am leaving all the old code in there for sending MWI for comparison purposes and so the other channels still compile.  I have no plans of taking it out since I run ast_data which doesn't load traditional peers into ram, hance my system doesn't slow down, even with 100K users and routes.  I figured this would help Mark or whomever commits this to CVS, well, if it gets commited to CVS.  I also have nothing to test the other channels with (IAX2, MGCP, etc) so someone at Digium or another programmer will have to do that (basically just copy and paste the code into the other do_monitor()'s).

 * This patch has been updated to work with CVS as of the time of this post.

By: Michael Shuler (mikes2277) 2004-12-10 03:00:16.000-0600

Ok, the patch I just uploaded is a bit messed up.  I guess I don't know how to use diff worth a damn.  Here is what I'm running:

diff -N -u -p -r asterisk-cvs asterisk-cvs-MWI > MWI_Version_2.patch

Any help would be appreciated.

By: Kevin P. Fleming (kpfleming) 2004-12-10 09:29:50.000-0600

Don't use "-p" on your diff command. You also want to add "-x CVS" to your diff command, so that it won't compare the CVS directories and their contents.

By: Kevin P. Fleming (kpfleming) 2004-12-10 09:32:17.000-0600

There is another _huge_ reason to have a method to turn off MWI when there are no more unread messages: shared mailboxes. Nearly every one of our clients has mailboxes that are available via multiple phones (on separate line appearances), and it's important that if one person clears out the mailbox, the MWI on the other phones is removed.

Also, the Polycom and Cisco phones do _not_ automatically clear MWI just because you entered the voicemail system via their "voicemail" button; they only turn it off when the SIP server tells them to stop MWI.

By: Jeffrey C. Ollie (jcollie) 2004-12-10 10:43:20.000-0600

It would also be nice if we could factor out the ADSI voicemail functions into a separate module.  I don't (and don't have any plans to) run any ADSI phones so it would be nice if I didn't have to load the ADSI modules.

By: Olle Johansson (oej) 2004-12-12 02:57:38.000-0600

If we are going to do this right from a SIP standpoint, MWI should be CLIENT driven, not PBX driven. The SIP client subscribes for MWI notification and we send it as long as the subscription is valid... So I need the channel to be in control in order to be able to fix the SIP channel. We have several reports of error messages from various SIP devices when we send MWI notifications without any subscriptions, or accept subscriptions but send MWI notifications with another Call-ID.

By: Kevin P. Fleming (kpfleming) 2004-12-12 09:44:21.000-0600

As I mentioned in another bugnote, how would client-driven MWI work if you want to be able to use multiple mailboxes for a single phone? I've not yet seen any phone that support MWI subscription offer anything like that, but I can do it easily if the MWI is PBX-driven.

By: Olle Johansson (oej) 2004-12-12 14:59:37.000-0600

kpfleming: That's no problem with asterisk. You can set mailbox= to multiple mailboxes and still subscribe to mailbox status for that peer. mailbox= to multiple mailboxes works today.

By: Michael Shuler (mikes2277) 2004-12-12 20:49:39.000-0600

Unfortunatly not all SIP phones send out a SUBSCRIBE but some do.  I would guess that we would need to make an option in the cahnnel's config (i.e. sip.conf) that sets the channel to not send out MWI unless a SUBSCRIBE is received first.  However their should be an option in sip.conf that allows MWI without the SUBSCRIBE first.

In regards to refresh I suppose we should also make this configurable via sip.conf for X min.  A thread will then be launched within the channel (or app, which is to be decided) to wake back up on a timer to send out any needed MWI.

Perhapse after this event system is done we should get this committed to CVS and then work on the rest of the larger problems that will be based off this new event system?

By: Michael Shuler (mikes2277) 2004-12-12 22:13:48.000-0600

Version 3 just got uploaded.  The diff still is a bit messy, I used:
diff -N -u -r -x CVS asterisk-cvs asterisk-cvs-MWI > MWI_Version_3.patch
Any ideas why its including full copies of app_voicemail.c?

Anyway, stuff that is new:
* Fixed a few silly bugs and typos

* Updated to the current CVS as of now

* Created GetMessageCount() in app_voicemail() to be the "offical" message counter.

* Consolidated transmit_notify_with_mwi() into sip_send_mwi_to_peer() since it was never called anywhere else.



Stuff that still needs to be done:
* GetMessageCount() for ODBC storage

* Remove ast_app_messagecount()... its used in manager.c (which I can't figure out what for) and in other channels.  I have no experience with any protos other than SIP so I need some help on the rest from anyone willing to patch chan_iax2, etc.  Anyone know what manager.c is all about?  There is a function in there referencing ast_app_messagecount() but that function is never referenced anywhere else in the source.



Once the above is done then I suggest this get commited to CVS and work be started on a periodic updater thread.  Unless Digium feels otherwise of course and wants the updater in this patch too, please let me know.

By: Michael Shuler (mikes2277) 2004-12-12 22:16:54.000-0600

Oops, forgot one more issue.  The system can no longer tell the difference between new and old messages for sending in NOTIFY.  Any suggestions or does this even matter?

The only suggestion I have is writing it into the voice mail description file but that seems a bit messy.

By: Jeffrey C. Ollie (jcollie) 2004-12-12 23:34:27.000-0600

> Version 3 just got uploaded. The diff still is a bit messy, I used:
> diff -N -u -r -x CVS asterisk-cvs asterisk-cvs-MWI > MWI_Version_3.patch
> Any ideas why its including full copies of app_voicemail.c?

You need to let CVS do the work for you:

cvs -d :pserver:anoncvs@cvs.digium.com:/usr/cvsroot co -d asterisk
cd asterisk
(edit... edit... edit...)
cvs diff -u

This should ignore all of the CVS/* stuff and the editor backup files.

By: Michael Shuler (mikes2277) 2004-12-13 00:45:25.000-0600

Thanks for the CVS diff tip, as you can see it worked :)

OK, to the best of my knowledge everything is done and bug free, cross your fingers :)

All that is left is the stuff I listed before that I could not do, or rather feel that there is someone else more qualified, i.e. markster to do it (other channels and removing the old app.c counter).  And, of course, Digium's offical OK on putting this into CVS... After that I would like to start a new "bug" thread for discussion on periodic MWI sending unless Digium refuses to commit to CVS without it.  If someone from Digium could let me know I would appeciate it.  I have already started work on the threading system but it may be quite some time before its done, or just a few hours, you never know :)

edited on: 12-13-04 00:46

By: Olle Johansson (oej) 2004-12-13 01:39:44.000-0600

I would recommend sending out mail to the -dev list asking people to test this and add their comments to the bug tracker.

Also, MWI checking periods is now configurable in sip.conf, I fixed that a while ago. Read sip.conf.sample for more information.

/O

By: Olle Johansson (oej) 2004-12-13 01:43:18.000-0600

Also, read the code guidelines in the doc directory. You should never use // style comments, only /* */.

By: Kevin P. Fleming (kpfleming) 2004-12-13 09:17:09.000-0600

oej: regarding multiple mailboxes on a single phone, I didn't describe myself properly. Here's the situation:

User has a Polycom SoundPoint IP 600 phone, which has six line appearances.

Two line appearances are configured for SIP peer phoneA.personal, which is the person's DID target, and mailbox 1234@company.

Two line appearances are configured for SIP peer phoneA.queue, which is where app_queue calls are delivered when this person is logged in, and mailbox sales@company. This mailbox also appears on other phones.

Two line appearances are configured for SIP peer phoneA.operator, which is where "operator" requests from the IVR for this company are delivered, and mailbox operator@company. This mailbox also appears on other phones.

The phone is not (as far as I am aware, cannot) set up to use SUBSCRIBE at all; we use entirely PBX-based MWI. The "voicemail" call button is configured to dial a particular feature code, and is the same for all line appearances. The diaplan then figures out the appropriate mailbox for that caller, and sends them to VoicemailMain.

If there's a way to accomplish this using SUBSCRIBE and _without_ requiring any mailbox knowledge to be configured into the phone, I'm all for it. I just don't see a way it could work, though, since every phone I've seen that supports SUBSCRIBE doesn't have separate settings for each line appearance.

By: Jeffrey C. Ollie (jcollie) 2004-12-13 11:58:00.000-0600

This patch doesn't really seem to create an "event based" system.  It merely replaces one polling system with another, since you have to scan the list of SIP peers every time that a task is placed on the queue.

I think that it would work better to have a way for a peer to register a callback with the voicemail system.  When a new message is left in a mailbox, the voicemail system would execute any callbacks that had been registered for that mailbox.  Also, callbacks would be executed when a mailbox had no more new messages.   There would also be a background thread that periodically refreshed the MWI status in case a notification had been missed.  Checks could also be done when a phone registered or became reachable after being unreachable, etc.

In the patch you use the function "ast_find_peers_with_mailbox".  It's obvious what that function _should_ do from the name, but I couldn't find it's implementation anywhere.  Is that part of the ast_data patch?  Is the ast_data patch being considered for inclusion in CVS HEAD?

By: Michael Shuler (mikes2277) 2004-12-13 18:37:23.000-0600

The ast_data stuff is referenceing the ast_data patch located at http://svn.asteriskdocs.org/res_data/ which I use quite extensivly.

Yes, I understand it's not fully event based but I wrote this patch for my purposes which is a large scale carrier deployment.  I agree that in a smaller enviornment that registering the peers with the voicemail system would work slightly better and make it fully event based, however the real purpose of this patch was to alieviate having to scan every mail box every second for new messages.  With 100K+ VoIP lines Asterisk would never be able to process any SIP messages because it was spending too much time messing with the spool dir and scanning through its own peer list.  I call it an event system because it never touches the mailboxes until something changes in that speciffic mailbox.  For me, and anyone else who runs the ast_data patch would need it to be structured this way because then it only has to call a simple SQL query to find all peers with mailbox=X.  This is a very fast indexed query which happens ONLY when a MWI status changes.  It then procedes to send out a MWI only to the phones that are subscribed to the mailbox.  The other way would not play nicely with ast_data because people usng it are trying to cluster for reliability and scalability.  A peer couldn't register with one Asterisk box, all have to be aware of what is going on but only one should respond.  Could get quite messy trying to coordinate who is the master MWI notifier vs creating MWI notification "tasks/events" on the server in the cluster that just manipulated the message.  I think this approach benifits both ast_data and non-ast_data users.

As far as ast_data being part of CVS... I would love to see it, I don't understand why it already isn't.  Why the "realtime" patch that isn't even realtime was added but ast_data wasn't is beyond me.  As far as reliability, ast_data has been running on my system since day 1 (about a year) without a single outage thanks to its natural clustering capabilities.  The only thing I would change in ast_data is to simplify it down to just an ODBC driver.  Since ODBC performance is much better than it used to be there is really no reason not to use just it as the driver.

By: Michael Shuler (mikes2277) 2004-12-13 19:21:57.000-0600

* Fixed "//" to "/* */"

* Changed functions to inline since they are very small

* sync with current CVS

By: Michael Shuler (mikes2277) 2004-12-13 22:57:47.000-0600

It occoured to me later that you referenced ast_find_peers_with_mailbox() which is actaully a function in a patch for ast_data to support the new MWI system.  Guess you won't find it in the "offical" release of ast_data :)  

And now for the full patch with ast_data and the MWI patch all in one.

By: Michael Shuler (mikes2277) 2004-12-16 20:37:55.000-0600

Fixed where I had mailbox and context backwards in a few places.  Uploaded as ast_data_MWI_2.patch and MWI_Version_6.patch.

So is this ever going to make it into CVS?  It reduces CPU and disk utilization quite a bit, I can't see why not.  Anyone at Digium paying attention to this bug/feature anymore?

By: Rob Gagnon (rgagnon) 2004-12-16 20:52:09.000-0600

I like the theory behind this.  I could never understand why this functionality was stuck into the do_monitor() function in the first place, so I can see how this can reduce load on a system with a large number of SIP subscribers.

By: drmac (drmac) 2004-12-17 09:45:55.000-0600

I don't believe these conform to the coding guidelines:

+// ---------------------------------------------------------------------------
+


Also, why all that native ODBC stuff? It defeats the purpose of having RealTime doesn't it?

IMHO it seems that all the ODBC code could be replaced with a few ast_load_realtime calls couldn't it? Especially now that RealTime supports more than just 1 operator on the select query.

By: Michael Shuler (mikes2277) 2004-12-17 11:14:42.000-0600

The ODBC stuff is for the ODBC voicemail storage patches.  It has nothing to do with anything that realtime would be a part of.

By: Mark Spencer (markster) 2004-12-17 11:54:39.000-0600

The concept isn't bad, but there are a lot of details that I think still should be worked out.

1) There is already an internal event system used for within Asterisk (not to mention the manager event system).  I don't want to have yet another one, so please try to find a way to reuse it.

2) Can we please leave function names and coding styles like the rest of Asterisk?  There are no filenames in Asterisk with capital letters.

By: Michael Shuler (mikes2277) 2004-12-17 14:05:52.000-0600

Mark, can you give me some filenames and function names where I can see this event system?  I haven't found the system you refered to earlier that would apply to this.

By: Paul Cadach (pcadach) 2004-12-21 11:45:49.000-0600

markster: Current event system needs slight re-design to allow passing some sort of information elements together with control/informational events. This could provide a way to make transparency for passing PRI/SS7 IEs along with control messages within Asterisk core.

By: habakuk (habakuk) 2004-12-31 13:40:57.000-0600

Am I missing something? It looks like your patch is missing MWI.h and MWI.c. A suggestion is to use:

diff -Naur asterisk-cvs asterisk-cvs.mwi

Also, any chance of creating a patch to work on 1.0 release?

By: Michael Shuler (mikes2277) 2005-01-04 00:24:34.000-0600

OK, version 7 I think is my final version.  It is synced with the latest CVS and makes use of the new voice mail registration functions that appeared in CVS recently.  It also includes a messagecount() ODBC function which was not present and really belongs as part of the ODBC patches, but I can't find them in CVS and without it my MWI code breaks.  One more function that needs to be done for ODBC is the hasvoicemail function.  Basically someone could just copy my messagecount function and modify a few lines.  Anyway, formatting has been changed through just about the whole thing to be consistant with the rest of Asterisk.  Also, someone needs to copy & paste over the do_monitor code from chan_sip into chan_iax2, chan_mgcp, etc. and test it (I have none of these type of devices).  Mark, please add to CVS when you get a chance or let me know what else you need me to do.

By: Trevor Peirce (trev) 2005-01-04 01:27:28.000-0600

Anything special that needs to be done to activate this?  The patch applied cleanly but on our sip phones they aren't being told about messages.

By: drmac (drmac) 2005-01-04 08:14:22.000-0600

version 7's patch file is not in unified format (-u) as per the patch making guides.

By: drmac (drmac) 2005-01-04 08:31:41.000-0600

i know im being nitpicky, but is there some reason for the inconsistant use of tabs/spaces? the code is hard to read because some lines are indented with tabs and some indented with spaces.

By: drmac (drmac) 2005-01-04 08:52:03.000-0600

kpfleming seems to have things right. it seems to me that a MWI should only be sent in 3 cases: phone registration, new/updated voicemail:

when someone leaves you a new voicemail, send the MWI packet.
when you delete/move a voicemail, send the MWI packet.
when your phone registers, check messages and send MWI packet.

make an app called "refreshMWI". this would be a very simple app that either scans the /voicemail/%/% paths or queries the database. Call the app via pbx_exec inside the chan_*.c file after the phone has registered. if it can't find the app, then ok, continue.

when someone leaves a new voicemail, call 'refreshMWI'.
After you have deleted/moved your new/old messages, call refreshMWI.

this seems like a whole bunch of code for something simple. or maby i am oversimplyfing things..

By: Michael Shuler (mikes2277) 2005-01-04 20:59:13.000-0600

The tabs and spaces things happens cause I use SecureCRT and when I copy and paste code between windows it copies tabs as spaces.  I agree, pain in the buttocks but other than fixing by hand I know no other way to do it... and since I don't get paid to write patches for Asterisk it is what it is ;)

diff -u on its way in a few min.  I didn't see that in the docs.

> when someone leaves you a new voicemail, send the MWI packet.

It does

> when you delete/move a voicemail, send the MWI packet.

It does

> when your phone registers, check messages and send MWI packet.

Dosen't do that.  Could be easily added, but I don't really need it for my purposes so feel free to add it (as a seperate patch later).  Since my phones register every 60 seconds to keep NAT bindings in place I actually don't want that behavior do to server loading as users and mailboxes increase.  For that matter (in my setup) REGISTER messages never reach Asterisk, SER does that for me.

By: Michael Shuler (mikes2277) 2005-01-04 21:07:40.000-0600

trev, nope, it should just magically work.  Turn on full console debugging, its quite verbose about telling you whats going on.  I tested it with and without ODBC voicemail storage support.  Its running live in my network with several thousand lines and appears to be stable and reliable.  I did notice that todays CVS causes a slight conflict with the patch.  I will put up another post later tonight.

By: Michael Shuler (mikes2277) 2005-01-04 21:25:47.000-0600

9 works with CVS as of 1/4/2005 at about 9:00 PM central time.

By: Trevor Peirce (trev) 2005-01-04 23:09:10.000-0600

Hmm,


CLI> set verbose 1000
Verbosity was 3 and is now 1000
CLI> set debug 1000
Core debug was 0 and is now 1000
<snip>
   -- Executing VoiceMail("SIP/mycompany701-fd15", "u703@mycompany") in new stack
   -- Playing 'voicemail/mycompany/703/greet' (language 'en')
   -- Playing 'vm-isunavail' (language 'en')
   -- Playing 'vm-intro' (language 'en')
   -- Playing 'beep' (language 'en')
   -- Recording the message
   -- x=0, open writing:  /var/spool/asterisk/voicemail/mycompany/703/INBOX/msg0002 format: wav49, 0x9a7aef0
   -- x=1, open writing:  /var/spool/asterisk/voicemail/mycompany/703/INBOX/msg0002 format: gsm, 0x9a7b088
   -- x=2, open writing:  /var/spool/asterisk/voicemail/mycompany/703/INBOX/msg0002 format: wav, 0x9a7b210
   -- User hung up
   -- SIP Seeding 'mycompany701' at mycompany701@1.2.3.4:5060 for 3600
<snip>

I'm using ODBC to store both realtime sip and voicemail information.  Version 10 of your patch was applied... is there anything I can do to figure out why it's not doing anything?

If I move the SIP configuration back into sip.conf everything works nicely so I know it's not the phone.

By: Michael Shuler (mikes2277) 2005-01-04 23:15:13.000-0600

put this in your logger.conf instead of what is there now:
console => notice,warning,error,debug,verbose

FYI: I'm on IRC asterisk-dev, handle mikes2277 now

edited on: 01-04-05 23:15

By: Michael Shuler (mikes2277) 2005-01-05 01:10:25.000-0600

Added MWI patch with ast_data and extra patches to ast_data to support MWI (which is broken in ast_data without this patch).  Mark, I would suggest you take a good look at ast_data in addition to my MWI patch.  Together they beef up Asterisk to take on Broadsoft and other such carrier systems.  No longer will Asterisk get progressivly slower as more users and mailboxes are added with these two patches.

By: Michael Shuler (mikes2277) 2005-01-05 02:48:20.000-0600

Well it seems that some of the ODBC stuff got broken in CVS recently.  Version 11 now includes a has_voicemail() and messagecount() for ODBC.  This fixes the problem of message counts being incorrect on the first message.

edited on: 01-05-05 02:55

By: Trevor Peirce (trev) 2005-01-05 02:50:32.000-0600

Ok, still not working but I have some better console output for you... note 1.2.3.4 is the public IP, and 4.3.2.1 is the unit's private IP behind the NAT router... just incase that makes any difference here.

Jan  5 00:38:36 DEBUG[4804]: app_voicemail.c:1789 sendmail: Sent mail to a@b.ca with command '/usr/sbin/sendmail -t'
Jan  5 00:38:36 DEBUG[4804]: app_voicemail.c:3262 notify_new_message: Inserting MWI NOTIFY Task For Mailbox: 701@mycompany (New 2, Old 12)
   -- Executing Hangup("SIP/mycompany701-5ebf", "") in new stack
 == Spawn extension (trevsip_go, 7701, 104) exited non-zero on 'SIP/mycompany701-5ebf'
Jan  5 00:38:36 DEBUG[4804]: cdr_addon_mysql.c:178 mysql_log: cdr_mysql: inserting a CDR record.
Jan  5 00:38:36 DEBUG[4804]: cdr_addon_mysql.c:197 mysql_log: cdr_mysql: SQL command as follows:  <snip>)
Jan  5 00:38:36 DEBUG[4804]: chan_sip.c:2213 sip_hangup: update_user_counter(mycompany701) - decrement inUse counter
   -- SIP Seeding 'mycompany701' at mycompany701@1.2.3.4:5060 for 3600
Jan  5 00:38:36 DEBUG[4804]: chan_sip.c:1369 __sip_ack: Stopping retransmission on '1868c402-fdfdd678@4.3.2.1' of Request 102: Found
Jan  5 00:38:36 DEBUG[4804]: chan_sip.c:8712 do_monitor: Processing MWI NOTIFY Task For Mailbox: 701@mycompany

Nothing else happens after this point.  This is with your latest patch.  Seems you have a lot of new files though, the realtime engine I'm using is configured from extconfig.conf and res_odbc.conf - it's already in current CVS - so I'm not sure what all these others are about.

I see on the phone I've put back into sip.conf this line shows up --

Jan  5 00:48:40 DEBUG[4804]: chan_sip.c:8743 do_monitor: Sending Peer (mycompany703) Notification For Mailbox: 703@mycompany (1/11)

-- but not for any peers defined through ODBC via extconfig.

By: Michael Shuler (mikes2277) 2005-01-05 03:03:05.000-0600

Try this patch, if you only have one NEW message it will not work without version 11 until you leave the second NEW message.  Old messages wont trick it into working.  The problem was actually in the ODBC storage stuff.

If you're not doing ODBC storage then try a SIP DEBUG from the console and see what IP/PORT its trying to send the NOTIFY to.  I bet its sending it to the wrong IP/PORT (nothing to do with this patch).

I also don't use extconfig and have no real way of testing it without a ton of work.  Does it output anything for extconfig clients?  It looks to me like the "realtime" stuff doesn't create traditional peer list entries.  There is no real documentation and there is nothing in the do_monitor() function for realtime's MWI system even before my patch.  So basically realtime most likely didn't do MWI before and it still won't until whomever the realtime author is gets inspired to do it cause it would take me days to figure out how it works (Asterisk isn't very well commented in the source).

The "extra" files for the ast_data patch is for a truely realtime config live from a SQL database.  The Asterisk "realtime" system is not actually realtime.  Basically ast_data is what realtime should have been (the name is very misleading).

edited on: 01-05-05 03:06

edited on: 01-05-05 03:08

edited on: 01-05-05 03:12

By: Kevin P. Fleming (kpfleming) 2005-01-05 07:52:58.000-0600

Your assumption is correct, the "realtime" peers are never included in the in-memory list (except when they have active calls), and thus are not available for do_monitor to send MWI to. Your patch is not a regression in that area.

By: Trevor Peirce (trev) 2005-01-05 13:23:20.000-0600

Ah, I must have misunderstood the intentions of this patch.  I thought it was changing around the way the whole system works (true) and part of that resolving the current issue that MWI does not work when using the realtime part of extconfig  (false) (ie. dynamically load in sip peers one by one as they are used, not just refresh it's internal sip list on reloads).

By: Michael Shuler (mikes2277) 2005-01-05 13:49:18.000-0600

trev, if you use the ast_data patch and use ast_data instead of realtime you will get the results you are looking for.  This patch works with the sip.conf and the sipfriends table in ast_data.  Realtime has its own set of problems that this patch doesn't adress, sorry.

By: raiden (raiden) 2005-01-18 02:17:32.000-0600

Any new progress on getting this MWI patch(or someone elses MWI patch) working with realtime? This seems to be a serious problem that needs to get fixed. (and included in CVS)

By: Michael Shuler (mikes2277) 2005-01-18 12:07:28.000-0600

Mark is working on it.  From what I understand he is taking the basis of this patch and making it work with some event system that already exists in Asterisk.

By: badelson (badelson) 2005-02-10 12:23:18.000-0600

does anyone have a current version of the patch that applies to the latest CVS?

By: nick (nick) 2005-03-09 21:21:29.000-0600

OK kids -- updates please -- is this going anywhere?

Nick

By: Donny Kavanagh (donnyk) 2005-03-09 22:15:21.000-0600

MWI is still broken (at least as of CVS about 1 1/2 weeks ago) with realtime *UNLESS* you set rtcachefriends=yes in your sip.conf, this then populates the * internal sip/peers list with the sip phones as they register.  This is not really a solution however, as the idea of realtime was to not populate these lists.  Thus, voicemail/mwi still needs work to rectify this.

edited on: 03-09-05 22:42

By: Kevin P. Fleming (kpfleming) 2005-03-10 00:56:31.000-0600

That is correct; at VON Mark and I talked about some possible solutions to these problems, and I've started putting together a potential generic event system for Asterisk. Given that I just returned home an hour ago and I'm not coherent (<G>), I don't expect to be able to post much for a few days. When I do post it, though, it will be on asterisk-dev and I'll need "all hands on deck" to peruse the code and try to poke holes in the concepts/implementation.

By: Michael Shuler (mikes2277) 2005-03-10 12:38:51.000-0600

Update this ticket to let me know when the new event system is done.  I will then rewrite my MWI patch around it.

By: Chris A. Icide (cicide) 2005-06-25 13:31:39

Where does this stand?

By: Michael Shuler (mikes2277) 2005-06-25 16:24:32

Mark doesn't like it and I got tired of updating it to the latest CVS.  I tried to pay Mark to come up with a solution that he liked along the same lines but he claimed that realtime "caching" fixed the problem.  It of course doesn't since all it does is the same thing as it did before except that realtime stores sipfriends in memory now which kindof defeats the purpose of realtime if you ask me...  Anyway I got tired of calling/emailing Mark begging for him to put my code or come up with another solution so I stopped maintaining it and instead wrote "Asterisk" like features for SER whos MWI system already scales quite nicely.

By: Michael Jerris (mikej) 2005-06-26 01:10:32

mikes2277, are you abandoning this bug for now?  Do you still wish to follow up once the event system is in?

By: Michael Shuler (mikes2277) 2005-06-27 22:42:31

If I have free time I suppose I will.  Who is working on the event system and how far are they away from being done?

By: Michael Jerris (mikej) 2005-06-27 22:45:51

kpfleming is working on the event system.  I will leave it to him to respond on eta.

By: Michael Jerris (mikej) 2005-07-20 19:14:52

4403\4592 have a fix for mwi w/ odbc in it.  It seems that this has stalled in the mean time waiting for the event system.  I am going to suspend this bug, can you please bring it back up when you are ready to proceed\asterisk is ready for this.