No subject


Fri Sep 2 03:59:05 CDT 2011


arning
and not an error. I take it you mean one that was already moved out of the =
inbox
in the older version then. What overall is the impact of this warning? It l=
ooks
like we are looking at a separate bug from the whole concept of adding mess=
age IDs
as needed. Instead, this appears to be a problem where we haven't accounted=
 for
the possibility of having a field with a NULL value when loading a message =
under
some circumstance.

Try changing the condition above the code that generates this error from:
            if ((res !=3D SQL_SUCCESS) && (res !=3D SQL_SUCCESS_WITH_INFO))=
 {
to:
            if ((res !=3D SQL_SUCCESS) && (res !=3D SQL_SUCCESS_WITH_INFO) =
&& (res !=3D SQL_NO_DATA))

and then see how that impacts this warning. I don't really think it's actua=
lly
impeding the operation of voicemail, but I could be mistaken I suppose.

I suppose if msg_id isn't your last column this might prematurely exit the =
loop
which could be rather bad...


- jrose


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


On Dec. 3, 2012, 11:22 a.m., jrose wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2220/
> -----------------------------------------------------------
> =

> (Updated Dec. 3, 2012, 11:22 a.m.)
> =

> =

> Review request for Asterisk Developers, Mark Michelson, Matt Jordan, and =
kmoore.
> =

> =

> Summary
> -------
> =

> When a message is moved between folders (such as after listening to new m=
essages, they are switched to the old messages folder), the msg_id field ad=
ded in Asterisk 11 isn't copied. This patch simply adds that field to what =
is copied in the odbc COPY function and appears to solve the problem.
> =

> =

> This addresses bug ASTERISK-20717.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20717
> =

> =

> Diffs
> -----
> =

>   /branches/11/apps/app_voicemail.c 376833 =

> =

> Diff: https://reviewboard.asterisk.org/r/2220/diff
> =

> =

> Testing
> -------
> =

> Made messages in a mailbox using app_voicemail. Checked mysql to see the =
msg_id. Verified an ID was created.
> Listened to messages, made sure the COPY function was invoked. Checked my=
sql again and saw that the IDs were gone.
> =

> Applied the patch, repeated the process, the IDs remained intact.
> =

> =

> Thanks,
> =

> jrose
> =

>


--===============3288621992293779053==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable




<html>
 <body>
  <div style=3D"font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor=3D"#f9f3c9" width=3D"100%" cellpadding=3D"8" style=3D"bor=
der: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href=3D"https://reviewboard.asterisk.org/r/2220/">https://reviewbo=
ard.asterisk.org/r/2220/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; padd=
ing-left: 10px;">
 <p style=3D"margin-top: 0;">On November 29th, 2012, 6:34 p.m., <b>Alec Dav=
is</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This does=
 fix the msg-id is lost problem when a message is &#39;moved&#39; from inbo=
x to old, or any other folder move.

I havn&#39;t followed this code path, but if you copy (note not move) a mes=
sage from 1 mailbox to another (if that&#39;s possible), then we&#39;d have=
 2 messages with the same msg_id.

Does msg_id need to be unique?
</pre>
 </blockquote>




 <p>On November 29th, 2012, 7:43 p.m., <b>Matt Jordan</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes it do=
es.  A copy should result in a new msg_id.</pre>
 </blockquote>





 <p>On November 30th, 2012, 10:28 a.m., <b>jrose</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">We need t=
o come up with a new method for generating the msg_id then, because the one=
 we have won&#39;t really cut it if uniqueness is necessary accross all mai=
lboxes. The second half of the string is just a string hash value for a seq=
uence of the extension, context, and the callerid used for the channel that=
 called voicemail, so that is going to stay the same during a copy. The fir=
st half is just the epoch time (seconds). If you have a function that copie=
s a message to twice at roughly the same time, that isn&#39;t going to work.

So for reference, the current msg_id string is...

(A)-hash((B)(C)(D))

where A is a timestamp unique to each second, B is call_extension, C is cal=
l_context, and D is call_callerid.

We could reasonably guarantee uniqueness if we simply used an atomically in=
creasing counter (like how we handle numerous channel names currently) in a=
ddition to the epoch time, so I&#39;m thinking the new msg_id string could =
be...

(U)-(A)-hash((B)(C)(D))

where U is the incrementing counter and everything else is the same. When d=
oing a copy, the timestamp becomes the time at copy.

That probably isn&#39;t enough though still. Multiple instances of Asterisk=
 could be using the database and presumably both could do a copy at the sam=
e time to the same message. Might as well throw systemname of whichever Ast=
erisk instance did they operation in there for good measure.

(S)(U)-(A)-hash((B)(C)(D))

We might need to increase the character count if we do this. systemnames us=
ually aren&#39;t more than 20 characters or so, but adding that to the uniq=
ue ID (8 hex characters), and the existing stuff (which is usually right ar=
ound 20 caracters right now and could be larger under certain conditions), =
the current 40 that we suggest on wiki and in the contrib script would prob=
ably be inadequate. Alternatively we could just scrap the call_extension, c=
all_context, and call_callerid hash from the message since it would no long=
er be necessary to provide uniqueness. We could also convert the timestamp =
into hex to save some characters.</pre>
 </blockquote>





 <p>On November 30th, 2012, 10:44 a.m., <b>jrose</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looks lik=
e we actually set aside 128 characters for system name.  Yuck.  Maybe I sho=
uld hash that and write as hex too.</pre>
 </blockquote>





 <p>On December 3rd, 2012, 10:30 p.m., <b>Alec Davis</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">found: ht=
tp://linux.die.net/man/3/uuid_generate

uuid_generate, uuid_generate_random, uuid_generate_time, uuid_generate_time=
_safe - create a new unique UUID value =


The UUID is 16 bytes (128 bits) long, which gives approximately 3.4x10^38 u=
nique values (there are approximately 10^80 elementary particles in the uni=
verse according to Carl Sagan&#39;s Cosmos). The new UUID can reasonably be=
 considered unique among all UUIDs created on the local system, and among U=
UIDs created on other systems in the past and in the future. =


Maybe something in this as to what it&#39;s using for randomness.</pre>
 </blockquote>





 <p>On December 3rd, 2012, 10:41 p.m., <b>Alec Davis</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">perhaps t=
he work has already been started/done https://reviewboard.asterisk.org/r/22=
17/</pre>
 </blockquote>





 <p>On December 4th, 2012, 9:46 a.m., <b>jrose</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">We can&#3=
9;t use that in 1.8 unfortunately. Matt and I talked about this and its imp=
ortant to keep it either the same or similar to the current ID, at least as=
 far as the appearance of the ID is concerned.</pre>
 </blockquote>





 <p>On December 4th, 2012, 10:09 a.m., <b>Matt Jordan</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It wouldn=
&#39;t be 1.8, it&#39;d actually be in 11.

The reason for not making that change in release branches is to not introdu=
ce a library dependency in the middle of a version release (and, if the col=
umn length changes, potential schema modifications as well). As it turns ou=
t, that&#39;s less of a concern since pjproject depends on libuuid as well =
(as Russell discovered), so its possibly not as big of a show stopper - but=
 creating a dependency on a library mid-stream feels bad.</pre>
 </blockquote>





 <p>On December 4th, 2012, 10:13 a.m., <b>jrose</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">&gt; It w=
ouldn&#39;t be 1.8, it&#39;d actually be in 11.
Er, that&#39;s what I meant. </pre>
 </blockquote>





 <p>On December 18th, 2012, 8:03 p.m., <b>Alec Davis</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">asterisk-=
11 CHANGES.TXT
{code}
   All messages created in old Asterisk installations will have a msg_id ad=
ded to
   them when required. This operation should be transparent as well.
{code}

This isn&#39;t the case!</pre>
 </blockquote>





 <p>On December 18th, 2012, 11:08 p.m., <b>jrose</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I haven&#=
39;t really changed anything about the circumstances behind adding msg_ids =
to messages from old installations, so if is false with the patch, it&#39;s=
 false without the patch.  Can you please be a little more specific about t=
he scenario for which this isn&#39;t true? I&#39;ve tried using voice mails=
 recorded in old installations with this patch and they seem to work and re=
ceive the msg_id when they are read and moved as what you&#39;d generally e=
xpect to be required.

I think transparency in this case just means the end user doesn&#39;t have =
to be particularly involved with the process, but if that&#39;s what you me=
an please enlighten me.</pre>
 </blockquote>





 <p>On December 19th, 2012, 12:47 a.m., <b>Alec Davis</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">CHANGES.t=
xt was wrong before this patch, so you haven&#39;t broken anything as you s=
ay.

Now with the opening of a voicemail, they would generally be moved old, and=
 you patch will set a msg_id.

The issue we&#39;re seeing is the greeting, from an old asterisk install, i=
t didn&#39;t have a msg-id, and because it&#39;s not moved around, it will =
never get one.
Thus we always see the error &quot;SQL Get Data error! coltitle=3Dmsg_id&qu=
ot;

</pre>
 </blockquote>





 <p>On December 19th, 2012, 1 a.m., <b>Alec Davis</b> wrote:</p>
 <blockquote style=3D"margin-left: 1em; border-left: 2px solid #d0d0d0; pad=
ding-left: 10px;">
  <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-sp=
ace: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">does the =
mysql msg_id column need to change size for this patch?
If so what to?</pre>
 </blockquote>








</blockquote>

<pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-spac=
e: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">&gt; does t=
he mysql msg_id column need to change size for this patch?
&gt; If so what to?
As of the latest version of this patch, no, so nothing.

&gt; The issue we&#39;re seeing is the greeting, from an old asterisk insta=
ll, it didn&#39;t
&gt; have a msg-id, and because it&#39;s not moved around, it will never ge=
t one. Thus we
&gt; always see the error &quot;SQL Get Data error! coltitle=3Dmsg_id&quot;



More information about the asterisk-dev mailing list