Summary:	ASTERISK-01222: [patch] New API for modularizing external queries outside of Asterisk Base
Reporter:	Rob Gagnon (rgagnon)	Labels:
Date Opened:	2004-03-16 13:19:29.000-0600	Date Closed:	2011-06-07 14:05:32
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_sip.patch4.txt ( 1) qint_mysql.c ( 2) qint_mysql.c.pattern ( 3) qint_mysql.c.pattern2 ( 4) qint4.tar.gz
Description:	Introducing QINT... A Query Interface for Asterisk.  Eliminates all DB specific SQL code from chan_sip.c, app_voicemail.c, and establishes a method under which other flat files can be hooked to an external data source, whether its mysql, pgsql, informix, oracle, ldap, radius, or whatever.... ****** ADDITIONAL INFORMATION ****** Description of files in the tarball: qint.patch.txt: - cvs patch to change the following files (paths are relative to /usr/src/asterisk/) Makefile - changed to add compilation for qint.c asterisk.c - changed to call load_qint() inside of qint.c apps/app_voicemail.c - changed to remove all SQL, and call handlers in qint.c instead channels/chan_sip.c - changed to remove all SQL, and call handlers in qint.c instead contrib/scripts/sipfriends.sql - mods to support new fields contrib/scripts/vmdb.sql - cleans up SQL in original to make it readable New files in the tarbal: qint.c - The query interface itself. include/asterisk/qint.h -duh? include/asterisk/chan_sip.h - some struct definitions moved from chan_sip.c so qint can use them include/asterisk/app_voicemail.h - some struct definitions moved from app_voicemail.c so qint can use them qint/Makefile - Makefile for qint_xxx modules qint/qint_mysql.c - Existing mysql code moved out of other modules and put here configs/qint.conf.sample - what else? contrib/scripts/sip-acl.sql - Access Control List table to go with the queries in qint_mysql.c Usage: ------ I've included a working qint_mysql.c module.  Others can write qint_pgsql.c or whatever, but I dont use PostGres, so I didnt want to add in bad code. Now, chan_sip.c and app_voicemail.c will make calls to routines within qint.c as appropriate.  If a function is registered with qint.c to handle a query, it will call that function, which is responsible for populating the appropriate return values. Comments?
Comments:	By: Rob Gagnon (rgagnon) 2004-03-16 17:04:28.000-0600 I know more queries need to be pulled out to be complete:  IE: chan_iax?.c but I wanted to see some kind of comment before going and doing that. By: Brian West (bkw918) 2004-03-17 21:35:10.000-0600 bravo... but wouldn't a loadable module be more acceptable?  If I don't use it I don't want it there.  So maybe a res_qint (or res_sql because most will say WTF is QINT!?) By: Mark Spencer (markster) 2004-03-17 23:45:05.000-0600 This is obviously not a major bug.  Please to not label "features" as "major bugs". By: Rob Gagnon (rgagnon) 2004-03-18 13:27:31.000-0600 markster:  oops.. I couldnt find the right combination of major and feature to open under.  Will watch that in the future. bkw:  if qint.c was optional, the calls to the core routine to see if a registered module had to be called couldnt be there without a #ifdef check.  The technology here was borrowed partly from how cdr.c operates.  The qint_xxxx.c modules are completely optional.  For example, if you dont have qint_mysql.so in the /usr/lib/asterisk/modules directory, then it is ignored completely. Also, I went with QINT (Query Interface) because not all modules will go to a database. While in discussion on #asterisk, other mentioned LDAP and RADIUS would be nice to query.   Calling it dbint, or sqlint just didn't fit.  But the generic word "Query" fit a little better. On another note, I integrated the extensions.conf flat file into mysql, and now using qint to hit the database live for extension lookups. Of course, the only thing right now that can be in the table are exact matches, but that is mostly what people are going to want to put there.  The pattern matching, and global variables can stay in the flat file. edited on: 03-18-04 12:27 By: Rob Gagnon (rgagnon) 2004-03-30 16:10:18.000-0600 Latest file is the one dated "03-30-04 16:06"  (I forgot to use a new name.. sorry) Updated patch files for following reasons: Patch files are current for CVS as of this bugnote's timestamp. Found minor bugs in the Makefiles I added.  Fixed that. Added "extensions.sql" schema for creating a mysql table for qint_mysql.c support to run your extensions live out of a database instead of from extensions.conf Note that this is only valid for exact matches in the DB.  Pattern matches will need to remain in the extensions.conf file. By: Michael Shuler (mikes2277) 2004-03-30 17:54:51.000-0600 Have you though about using MySQL as just a place to store the config but to have Asterisk just check it every once in a while for changes (like 10 seconds or so) and then update the in memory tables.  That would give four advantages: 1 - It would improve the load on MySQL because a high volume server would only hit the database once every 10 seconds (or 5, 10, 15, 20 or whatever the admin wants). 2 - It would improve the performance of Asterisk because all lookups would be done in memory instead of waiting on an MySQL server. 3 - It would improve uptime on a cluster of Asterisk servers since having continuous access to a MySQL server would not be needed since everything would be stored in RAM (incase the MySQL server died). 4 - You could then have full pattern matching again because Asterisk is basically working as it did before. The only disadvantage would be that it would take a change in configuration X number of seconds to propagate through the cluster. By: Rob Gagnon (rgagnon) 2004-03-30 18:01:54.000-0600 yeah I thought about that, but for our system, which is expected to have 50,000+ users per server, I didnt want to worry about memory searching issues with the given linear linked-list system.   Also didnt want to worry about running out of memory (not that you cant buy more, but some systems will limit the ram per process) Generally only need the fixed extensions to be easily modifiable anyhow, as that is what end-users will want to change all the time. edited on: 03-30-04 16:53 By: Michael Shuler (mikes2277) 2004-03-31 19:45:54.000-0600 I have talked to markster about changing things to hashs or something more efficient instead.  We looked over RAM requirements for 100K users (my application) and didn't see it as much of an issue within 1GB of RAM. My application sounds very similar to yours.  Perhapse we can work together on it.  Contact me at mike@bwsys.net to discuss if you would like.  I would hate for us to end up writing the same thing twice ;) By: Michael Shuler (mikes2277) 2004-04-01 14:10:27.000-0600 When appling the patch to todays CVS I get patching file apps/app_voicemail.c patch: **** malformed patch at line 656: @@ -1360,12 +1257,6 @@ Any idea what changed? By: Rob Gagnon (rgagnon) 2004-04-01 16:15:43.000-0600 problem may have been changes to app_voicemail.c since my last patch, or could have been fact I manually editted the app_voicemail.c portion of the patch file because I did not want to submit two changes in one.  I have re-created the set of patches in qint3.tar.gz. Let me know if there are any other issues.  Thanks for the heads-up. On another note: This interface could allow for the memory refreshing version you talk about.  All that would be needed is a different qint_mysqlxxx.c module that would read the db periodically, and maintain extensions in memory instead of the version I have supplied, which performs an SQL for each lookup. The QINT technology here allows for the module to change its lookup functionality without modifying the chan_sip.c code to do it (for example) edited on: 04-01-04 15:09 By: Michael Shuler (mikes2277) 2004-04-01 19:36:18.000-0600 Everything is working great that I have tested so far (I have done nothing with VM yet).  The only strange but non critial issue I have seen is in my CDR records.  src and dst are correct but clid is "" <5555555555> and it used to be just 5555555555.  This seems to only be a problem with my Sipura originating calls.  Calls from my gateway to the Sipura don't seem to have any issues and are logged corretly as just 5555555555.  I don't use that field anyway but I thought it was interesting that it changed.  FYI: I am using cdr_odbc for my CDRs. By: Rob Gagnon (rgagnon) 2004-04-02 16:10:20.000-0600 mikes2277... Thanks for the offline tip on the segmentation fault you were getting. gdb revealed I got mud on my face checking a string length.  :-( I forgot to check the pointer was valid before sending it to strlen().  The newly uploaded qint_mysql.c file should resolve that issue. So, for anyone else, you will need to download the qint3.tar.gz file, and then replace the qint_mysql.c file within the tarball with the one uploaded here dated 04-02-04 16:07 the callerid info is displaying differently in the CDR's due to the method used reading caller ID info from the database.  For ease of parsing for other processes, I split the callerid field from sip.conf into two fields in the database. if you have "callerid=5555555555" in the sip.conf file, then you probably have cidname="" and cidnumber="5555555555" in the mysql database to try and be the equivalent. The two fields in the db change into: "[cidname]" <[cidnumber]> inside of qint_mysql.c for chan_sip.c.  Since your cidname is empty, you see: "" <5555555555> NOTE: that the "[cidname]" <[cidnumber]> format is the full caller id name and number syntax used by chan_sip.c for caller id. If a name were present in the db (IE: Mike), you would see: "Mike" <5555555555> for the callerid in your CDR's. I guess I could put a little more parsing on the read from the db to emulate the sip.conf entry exactly, but I was trying to just get things working at the time this was written. edited on: 04-02-04 15:30 By: Michael Shuler (mikes2277) 2004-04-06 12:03:50 patch fails on chan_sip.c with todays CVS. Would be nice if the whole patch was one big diff file because app_voicemail.h, chan_sip.h etc. change too. Thanks PS: I am still working on the pattern matching. By: Rob Gagnon (rgagnon) 2004-04-07 11:02:03 Thats odd, since app_voicemail.h and chan_sip.h are brand new files in this patch, that are not comitted to cvs, so there would be no diff to create for them. Also, I just did a plain old "cvs update" in /usr/src/asterisk/ on my test box, and there were no merge conflicts.  I can work on a more current tarball now. By: Rob Gagnon (rgagnon) 2004-04-07 15:50:22 ok,  qint4.tar.gz  is now the most recent version as of today's CVS. By: Michael Shuler (mikes2277) 2004-04-07 23:32:09 Thanks for the updated diff.  I noticed in the channels/Makefile there are 2 entries USE_SIP_MYSQL_FRIENDS and USE_MYSQL_FRIENDS.  Should both be enabled? By: Michael Shuler (mikes2277) 2004-04-08 00:06:44 I think something may be missing out of the diff because I can't seem to register anymore. By: Rob Gagnon (rgagnon) 2004-04-08 10:09:09 The MACRO's in the channels/Makefile have no effect on the patched chan_sip.c file.  I just chose not to remove the macros at this time since they are still used for the IAX stuff. I will look into the registration issues, but it was still working for me yesterday... hmmm.... Also, there may be a new "feature/bug" opened soon to replace this one, as the QINT name and methodology is going to change to be a "res" module after speaking with some people in #asterisk-dev. This will cause fewer changes to other core asterisk files.  IE: I won't need to move some structs out of the .c files they are in.  (Meaning that chan_sip.h and app_voicemail.h won't be needed). Don't freak out though, as the functionality will remain exactly the same, only the names will change (to protect the innocent :-) ) By: Michael Shuler (mikes2277) 2004-04-08 10:26:09 The patch doesn't seem to be doing anything to the chan_sip.c file so I'm guessing it just got missed by accident. I guess I will stop my development stuff I was working on until the new "res" stuff gets posted.  Let me know what the new bug # is when you get it going so I can check it out. By: Rob Gagnon (rgagnon) 2004-04-08 13:25:58 chan_sip.patch4.txt added... Whole patch needs the qint4.tar.gz file and the chan_sip.patch4.txt file now Somehow that one file was missed in the diff..  Sorry about that. By: Michael Shuler (mikes2277) 2004-04-09 20:25:32 Just uploaded qint_mysql.c.pattern which is a drop in replacement for the qint_mysql.c by rgagnon.  It now lets MySQL supports pattern matching.  I also added some comments to help others out. Rob, at the end of the mysql_ext_lookup function (which is all I changed) the very last return starting on line 288 may or may not be correct.  I didn't know what values were appropriate for errno when no match is found but records were found (pattern matches that didn't match).  Either way it works as is. By: Rob Gagnon (rgagnon) 2004-04-09 21:43:42 Cool thing on the pattern matching.  I will make sure to include that in the new res_data modules that are being worked on. Re: the errno stuff, you should be good.  I included the setting of errno for the future, but nothing in this implementation actually acts upon it. By: Rob Gagnon (rgagnon) 2004-04-09 21:56:41 Neat idea on matching by ordering ascii ascending on the extension.  One thing I would think should still be done, is to also order patterns descending by length since you want to have the best match possible. Imagine: _901144.,1,Dial(blah) _9011443.,1,Dial(otherblah....) A person dialing 9011443067723345 (not a real number) would end up dialing "blah" instead of the intended "otherblah" because the match on the shorter pattern would hit first. Kinda tricky to do with SQL directly, so some neat code will be needed. By: Brian West (bkw918) 2004-04-09 22:10:08 accually not tricky to do at all with SQL, maxlen and a second query does wonders. By: Michael Shuler (mikes2277) 2004-04-09 23:38:15 Well, the good news is that MySQL natural order when ASC is to put "_X." before "_XX.".  The problem starts to show up when you have "_NX." or "_NX." with the previous two.  Or even "_9X."... So the real question is which should come first?  Well, I don't see how Asterisk/MySQL could possibly make that decision because there is no way it could possibly know what the user truely wants. With that in mind I propose that we add another column i.e. "order" for controlling match attempt order.  If you don't care what order things get processed then just set them all to one.  But if you have a group of extensions or a single extension that you need to have checked first then make it/them all a 1 and the remaining a 2 or any other higher number.  That way all we need to do as add another column to the ORDER BY and then presto.  Anyone see any downfall to this approach? By: florian (florian) 2004-04-10 00:12:12 Uhm, question: What does asterisk do with such ambiguous numberplans at the moment (without QINT) ? It seems to me we should not put effort in making it possible to have more or less ambiguous dialplans by adding an order field if we can also design better dialplans ? By: Michael Shuler (mikes2277) 2004-04-10 00:17:26 How about this as a half way solution without actually having an "order" column: ORDER BY LEFT(extension, 1) ASC, CHAR_LENGTH(extension) DESC This will make specific extensions come first (stuff without an '_' in front of it) and then will make the most specific pattern match come first and the least specific pattern match come last. By: Michael Shuler (mikes2277) 2004-04-10 00:19:15 florian, to the best of my knowlege it makes comparisons in the order they appear in the extensions.conf file which is impossible to emulate in an SQL table without the "order" column I suggested above. By: Michael Shuler (mikes2277) 2004-04-10 00:30:47 For those who want the "ORDER BY LEFT(extension, 1) ASC, CHAR_LENGTH(extension) DESC" behavior use qint_mysql.c.pattern2. The more I though about it the more I realized that this behavior logicaly makes the most sense.  It behaves very similar to how a Cisco router would i.e. a /32 route would override a signle IP's behavior from a larger /24 route.  Most specific to least specific routing.  Well, I do believe we just made Asterisk smarter :) So there you go Rob, no tricky code required ;) By: Brian West (bkw918) 2004-04-17 22:53:07 I think res_data is going to replace this?  If not please let me know. Thanks, House Keeping By: Rob Gagnon (rgagnon) 2004-06-01 17:51:45 Just want to link this old version to the replacement which is much more robust. See ASTERISK-1731765 for ast_data's introduction.   Sorry to re-open, but this seemed to be only way to leave a trail from this bug to the new one. By: Olle Johansson (oej) 2004-06-02 04:08:51 Move to another bug report.