[Home]

Summary:ASTERISK-17299: [patch] Compile Error - odbc_storage enabled
Reporter:Michael L. Young (elguero)Labels:
Date Opened:2011-01-27 16:44:25.000-0600Date Closed:2011-04-01 05:38:12
Priority:MinorRegression?Yes
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) odbc_resequence_mailbox.diff
( 1) odbc_resequence_mailbox2.1.diff
( 2) odbc_resequence_mailbox2.diff
( 3) voicemail-compilation-error.diff
Description:After the changes were merged in r303679, there is a compile error with ODBC storage enabled.

****** ADDITIONAL INFORMATION ******

The closing tag needs to move up into the ifndef block.

Also with the closing tag moved, a compiler warning is generated since resequenced_mailbox() is not used at all with ODBC storage enabled.
Comments:By: Michael L. Young (elguero) 2011-01-27 17:01:35.000-0600

Woops... need to adjust the text in additional info:

"Also with the closing tag moved, a compiler warning is generated since resequence_mailbox() is not compiled in with ODBC storage enabled."

Should read:
"Also with the closing tag moved, a compiler warning is generated since resequenced_mailbox() is not used at all with ODBC storage enabled."
By: Michael L. Young (elguero) 2011-01-27 17:20:37.000-0600

Just a quick heads up for anyone seeing this.  There is a problem with ODBC storage not re-sequencing.

I tested the file based re-sequencing and that seems to be working.  When I just tried this out with ODBC, there is a problem.  Mailbox has one message but in the database it is msgnum 4.  Therefore, the message is not being retrieved.

I was able to reproduce this by following the steps outlined in issue ASTERISK-17292.  Listen to a new voice message, hit 9 and then 0 to save it to INBOX.  Hangup.  Try to call in again.  There is one message but it is not retrieved.

Will try to investigate a possible solution to this a little bit later.
By: Michael L. Young (elguero) 2011-01-29 21:37:32.000-0600

The uploaded patch 'odbc_resequence_mailbox.diff' resolves the compile error plus adds my proposal for having re-sequencing performed when using ODBC STORAGE.

This patch changes last_message_index to do what the name suggests.  It now actually returns the last_message_index for the dir named in the database instead of assuming that the count of messages is the same as the index number.

Also, count_messages will do just that; it will count the messages that are in the database.

I have tested this patch out and everything seems to be working as expected.
By: iasgoscouk (iasgoscouk) 2011-01-30 02:41:33.000-0600

thanks - app_voicemail now compiles for ODBC storage


By: Michael L. Young (elguero) 2011-01-31 14:23:35.000-0600

Looks like Tilghman took care of the compiler error earlier this morning.

I have updated the patch to basically enable the ODBC re-sequencing.
By: Kevin Scott Adams (nivek) 2011-03-22 10:35:03

I have been monitoring the app_voicemail issues for sometime now.  We finally ran into the issue with ODBC resequencing at a couple of our sites that are on 1.8.0.
We (Marquis and I) checked out a fresh copy of 1.8-branch and problem still exists.
The attached patch seems to correct the issue at hand and makes sense.
Even though the #ifdefs are change so resequence_mailbox is compiled for ODBC it gives the functions last_message_index and count_messages different personalities.  Before they were, basically, the same function so I believe the resequence_mailbox would never be called due to the 'if (vms->lastmsg != last_msg)' block.

My only question is...Should the resequencing be done on closing of the mailbox instead of opening of the mailbox or both?  With the 9 - 0 key sequence, the msgnum is still out-of-sequence upon closing.  Will this cause other issues like forwarding, new message, e.t.c.

I will test these conditions and let you know.

I also believe most of the voicemail resequencing issues are related.


By: Alec Davis (alecdavis) 2011-03-29 12:50:36

Being databased storage, we also need to consider that a message could be removed by an external source, where open_mailbox or close_mailbox are not involved.
By: Kevin Scott Adams (nivek) 2011-03-29 13:48:56

So if I delete a message from the SQL console, would that be considered an 'external source'?
By: Alec Davis (alecdavis) 2011-03-29 15:35:01

Yes, that should cover it, try deleting message 0 from the SQL console.
Now if the mailbox was resequenced on close_mailbox, the following wouldn't work;
wth message 0 deleted, go into voicemail and try to listen to the first message. It doesn't exist.

So if a resequence is needed it should be in open_mailbox, then the user won't experience a problem.

If the resequence was only in close_mailbox, it is then fixed so that next time the user goes in it's right, weird user experience!

But I do like the last_message_index change, as this will prevent further messages coming in being lost, instead of overwriting as it currently does.


By: Kevin Scott Adams (nivek) 2011-03-31 07:23:04

Completely understand.
Tested review board diff with 1.8.4.0 with no issues yet.  Looks good.
By: Michael L. Young (elguero) 2011-03-31 09:49:02

Thanks for the testing and helping to push this through.  Glad that it is working for everyone.
By: Digium Subversion (svnbot) 2011-04-01 01:46:59

Repository: asterisk
Revision: 312070

U   branches/1.4/apps/app_voicemail.c

------------------------------------------------------------------------
r312070 | alecdavis | 2011-04-01 01:46:56 -0500 (Fri, 01 Apr 2011) | 16 lines

app_voicemail: close_mailbox needs to respect additional messages while mailbox is open.

close_mailbox leave gaps in message sequence if messages are deleted and new messages
arrive during this time, this is because the shuffle down to slot 0, only shuffles
the number of pre-existing messages when mailbox is opened, ignoring new arrivals.

Fix: in close_mailbox re-evaluate number of messages before the shuffle, this then includes new arrivals.

Happens on filebased or ODBC storage.

(issues ASTERISK-17613,ASTERISK-17207,ASTERISK-17299,ASTERISK-17580)
Reported by: alecdavis,tootai,afosorio

Review: https://reviewboard.asterisk.org/r/1153/


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312070
By: Digium Subversion (svnbot) 2011-04-01 02:25:57

Repository: asterisk
Revision: 312103

_U  branches/1.6.2/
U   branches/1.6.2/apps/app_voicemail.c

------------------------------------------------------------------------
r312103 | alecdavis | 2011-04-01 02:25:55 -0500 (Fri, 01 Apr 2011) | 22 lines

Merged revisions 312070 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r312070 | alecdavis | 2011-04-01 19:46:56 +1300 (Fri, 01 Apr 2011) | 16 lines
 
 app_voicemail: close_mailbox needs to respect additional messages while mailbox is open.
 
 close_mailbox leave gaps in message sequence if messages are deleted and new messages
 arrive during this time, this is because the shuffle down to slot 0, only shuffles
 the number of pre-existing messages when mailbox is opened, ignoring new arrivals.
 
 Fix: in close_mailbox re-evaluate number of messages before the shuffle, this then includes new arrivals.
 
 Happens on filebased or ODBC storage.
 
 (issues ASTERISK-17613,ASTERISK-17207,ASTERISK-17299,ASTERISK-17580)
 Reported by: alecdavis,tootai,afosorio
 
 Review: https://reviewboard.asterisk.org/r/1153/
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312103
By: Digium Subversion (svnbot) 2011-04-01 02:32:15

Repository: asterisk
Revision: 312117

_U  branches/1.8/
U   branches/1.8/apps/app_voicemail.c

------------------------------------------------------------------------
r312117 | alecdavis | 2011-04-01 02:32:13 -0500 (Fri, 01 Apr 2011) | 29 lines

Merged revisions 312103 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r312103 | alecdavis | 2011-04-01 20:25:54 +1300 (Fri, 01 Apr 2011) | 22 lines
 
 Merged revisions 312070 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r312070 | alecdavis | 2011-04-01 19:46:56 +1300 (Fri, 01 Apr 2011) | 16 lines
   
   app_voicemail: close_mailbox needs to respect additional messages while mailbox is open.
   
   close_mailbox leave gaps in message sequence if messages are deleted and new messages
   arrive during this time, this is because the shuffle down to slot 0, only shuffles
   the number of pre-existing messages when mailbox is opened, ignoring new arrivals.
   
   Fix: in close_mailbox re-evaluate number of messages before the shuffle, this then includes new arrivals.
   
   Happens on filebased or ODBC storage.
   
   (issues ASTERISK-17613,ASTERISK-17207,ASTERISK-17299,ASTERISK-17580)
   Reported by: alecdavis,tootai,afosorio
   
   Review: https://reviewboard.asterisk.org/r/1153/
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312117
By: Digium Subversion (svnbot) 2011-04-01 02:43:03

Repository: asterisk
Revision: 312118

_U  trunk/
U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r312118 | alecdavis | 2011-04-01 02:43:00 -0500 (Fri, 01 Apr 2011) | 36 lines

Merged revisions 312117 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r312117 | alecdavis | 2011-04-01 20:32:12 +1300 (Fri, 01 Apr 2011) | 29 lines
 
 Merged revisions 312103 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r312103 | alecdavis | 2011-04-01 20:25:54 +1300 (Fri, 01 Apr 2011) | 22 lines
   
   Merged revisions 312070 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r312070 | alecdavis | 2011-04-01 19:46:56 +1300 (Fri, 01 Apr 2011) | 16 lines
     
     app_voicemail: close_mailbox needs to respect additional messages while mailbox is open.
     
     close_mailbox leave gaps in message sequence if messages are deleted and new messages
     arrive during this time, this is because the shuffle down to slot 0, only shuffles
     the number of pre-existing messages when mailbox is opened, ignoring new arrivals.
     
     Fix: in close_mailbox re-evaluate number of messages before the shuffle, this then includes new arrivals.
     
     Happens on filebased or ODBC storage.
     
     (issues ASTERISK-17613,ASTERISK-17207,ASTERISK-17299,ASTERISK-17580)
     Reported by: alecdavis,tootai,afosorio
     
     Review: https://reviewboard.asterisk.org/r/1153/
   ........
 ................
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312118
By: Digium Subversion (svnbot) 2011-04-01 03:29:51

Repository: asterisk
Revision: 312174

U   branches/1.4/apps/app_voicemail.c

------------------------------------------------------------------------
r312174 | alecdavis | 2011-04-01 03:29:49 -0500 (Fri, 01 Apr 2011) | 23 lines

voicemail: get real last_message_index and count_messages, ODBC resequence

change last_message_index to read the max msgnum stored in the database
change count_messages to actually count the number of messages.

last_message_index change:
 This fixed overwriting of the last message if msgnum=0 was missing.
 Previously every incoming message would overwrite msgnum=1.
count_messages change:
 allows us to detect when requencing is required in opneA_mailbox.
resequence enabled for ODBC storage:
 Assists with fixing up corrupt databases with gaps, but only when
 a user actively opens there mailboxes.

(closes issue ASTERISK-17299,ASTERISK-17207,ASTERISK-17613)
Reported by: elguero
Patches:
     based on odbc_resequence_mailbox2.1.diff uploaded by elguero (license 37)
Tested by: elguero, nivek, alecdavis

Review: https://reviewboard.asterisk.org/r/1153/


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312174
By: Digium Subversion (svnbot) 2011-04-01 03:47:31

Repository: asterisk
Revision: 312210

_U  branches/1.6.2/
U   branches/1.6.2/apps/app_voicemail.c

------------------------------------------------------------------------
r312210 | alecdavis | 2011-04-01 03:47:30 -0500 (Fri, 01 Apr 2011) | 29 lines

Merged revisions 312174 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r312174 | alecdavis | 2011-04-01 21:29:49 +1300 (Fri, 01 Apr 2011) | 23 lines
 
 voicemail: get real last_message_index and count_messages, ODBC resequence
 
 change last_message_index to read the max msgnum stored in the database
 change count_messages to actually count the number of messages.
 
 last_message_index change:
   This fixed overwriting of the last message if msgnum=0 was missing.
   Previously every incoming message would overwrite msgnum=1.
 count_messages change:
   allows us to detect when requencing is required in opneA_mailbox.
 resequence enabled for ODBC storage:
   Assists with fixing up corrupt databases with gaps, but only when
   a user actively opens there mailboxes.
 
 (closes issue ASTERISK-17299,ASTERISK-17207,ASTERISK-17613)
 Reported by: elguero
 Patches:
       based on odbc_resequence_mailbox2.1.diff uploaded by elguero (license 37)
 Tested by: elguero, nivek, alecdavis
 
 Review: https://reviewboard.asterisk.org/r/1153/
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312210
By: Digium Subversion (svnbot) 2011-04-01 04:03:14

Repository: asterisk
Revision: 312211

_U  branches/1.8/
U   branches/1.8/apps/app_voicemail.c

------------------------------------------------------------------------
r312211 | alecdavis | 2011-04-01 04:03:12 -0500 (Fri, 01 Apr 2011) | 36 lines

Merged revisions 312210 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r312210 | alecdavis | 2011-04-01 21:47:29 +1300 (Fri, 01 Apr 2011) | 29 lines
 
 Merged revisions 312174 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r312174 | alecdavis | 2011-04-01 21:29:49 +1300 (Fri, 01 Apr 2011) | 23 lines
   
   voicemail: get real last_message_index and count_messages, ODBC resequence
   
   change last_message_index to read the max msgnum stored in the database
   change count_messages to actually count the number of messages.
   
   last_message_index change:
     This fixed overwriting of the last message if msgnum=0 was missing.
     Previously every incoming message would overwrite msgnum=1.
   count_messages change:
     allows us to detect when requencing is required in opneA_mailbox.
   resequence enabled for ODBC storage:
     Assists with fixing up corrupt databases with gaps, but only when
     a user actively opens there mailboxes.
   
   (closes issue ASTERISK-17299,ASTERISK-17207,ASTERISK-17613)
   Reported by: elguero
   Patches:
         based on odbc_resequence_mailbox2.1.diff uploaded by elguero (license 37)
   Tested by: elguero, nivek, alecdavis
   
   Review: https://reviewboard.asterisk.org/r/1153/
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312211
By: Digium Subversion (svnbot) 2011-04-01 04:08:42

Repository: asterisk
Revision: 312212

_U  trunk/
U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r312212 | alecdavis | 2011-04-01 04:08:39 -0500 (Fri, 01 Apr 2011) | 43 lines

Merged revisions 312211 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r312211 | alecdavis | 2011-04-01 22:03:11 +1300 (Fri, 01 Apr 2011) | 36 lines
 
 Merged revisions 312210 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r312210 | alecdavis | 2011-04-01 21:47:29 +1300 (Fri, 01 Apr 2011) | 29 lines
   
   Merged revisions 312174 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r312174 | alecdavis | 2011-04-01 21:29:49 +1300 (Fri, 01 Apr 2011) | 23 lines
     
     voicemail: get real last_message_index and count_messages, ODBC resequence
     
     change last_message_index to read the max msgnum stored in the database
     change count_messages to actually count the number of messages.
     
     last_message_index change:
       This fixed overwriting of the last message if msgnum=0 was missing.
       Previously every incoming message would overwrite msgnum=1.
     count_messages change:
       allows us to detect when requencing is required in opneA_mailbox.
     resequence enabled for ODBC storage:
       Assists with fixing up corrupt databases with gaps, but only when
       a user actively opens there mailboxes.
     
     (closes issue ASTERISK-17299,ASTERISK-17207,ASTERISK-17613)
     Reported by: elguero
     Patches:
           based on odbc_resequence_mailbox2.1.diff uploaded by elguero (license 37)
     Tested by: elguero, nivek, alecdavis
     
     Review: https://reviewboard.asterisk.org/r/1153/
   ........
 ................
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=312212