Summary:ASTERISK-03943: [patch] realtime support for app_queue.c
Reporter:knielsen (knielsen)Labels:
Date Opened:2005-04-15 08:11:57Date Closed:2008-01-15 15:36:51.000-0600
Versions:Frequency of
Environment:Attachments:( 0) rt-queue-patch4-dist.txt
Description:This is a patch to app_queue.c that enables dynamic realtime for queues.
Queue parameters and member list for a queue are loaded from realtime
each time the Queue application command is called, so that updates are
available immediately without the need for an explicit reload.

Disclaimer has been faxed on April 15, 2005. The disclaimer concerns
Sifira A/S, which is the company that employs me and owns the copyright
for the code.


To enable the use of dynamic realtime for queues, first create the table
in the database. For MySQL:

   CREATE TABLE queues_table (
     id      INT(11) NOT NULL AUTO_INCREMENT,  /* Optional */
    category VARCHAR(128) NOT NULL,
    var_name VARCHAR(128) NOT NULL,
    var_val  VARCHAR(128) NOT NULL,
    PRIMARY KEY  (id)

Ensure a proper realtime driver configuration (eg. res_mysql.conf).

Add a suitable family in extconfig.conf:

   queue => mysql,mydb,queues_table

Finally add the following entry to queues.conf:

   realtime_family = queue

to tell app_queue.c to use the newly defined realtime family.

If a queue name is defined statically in queues.conf, it will not be
looked up in dynamic realtime.

After this, queues can be defined / modified in the queues_table and be
immediately available without reload. Queues are defined in the table
with the same parameters used in the config file. The `category' column
is the queue name, the `var_name' column is the parameter name, and the
`var_val' column is the parameter value. For example:

   INSERT INTO queues_table VALUES (1,'my-example-queue','strategy','roundrobin');    
   INSERT INTO queues_table VALUES (2,'my-example-queue','music','default');        
   INSERT INTO queues_table VALUES (3,'my-example-queue','timeout','10');            
   INSERT INTO queues_table VALUES (4,'my-example-queue','retry','0');              
   INSERT INTO queues_table VALUES (5,'my-example-queue','maxlen','0');              
   INSERT INTO queues_table VALUES (6,'my-example-queue','member','SIP/111@mysip');  
   INSERT INTO queues_table VALUES (7,'my-example-queue','member','SIP/222@mysip');
   INSERT INTO queues_table VALUES (8,'my-example-queue','member','SIP/333@mysip');

This database setup is chosen to resemble closely the one used in static
realtime, so you should be able to use the same rows for static and
dynamic realtime. For now I have chosen this for simplicity; another
attractive and just-as-easy-to-implement possibility would be to have
two tables: One `queue' table with one row per queue and one column for
each parameter; and one `queue_member' table with one row per member and
columns for member interface and penalty. The latter model is perhaps
prettyer from a relatinal database perspective.

Other than that I will add documentation to the Wiki (or another
appropriate place if/when this is accepted into CVS.

Comments welcome, of course.
Comments:By: Brian West (bkw918) 2005-04-16 11:58:43

This is actually done kinda wrong.  Shouldn't the column name match the var.. and the contents of the column be the value?  This is pretty much automagic right?


By: knielsen (knielsen) 2005-04-17 00:04:09

bkw918, not sure if I understand you right, but I think you were expecting a database model like this:

CREATE TABLE queue_table (
 strategy VARCHAR(32),
 music VARCHAR(128),
 timeout INT(11),
 retry INT(11),
 maxlen INT(11),

CREATE TABLE queue_member_table (
 queue_name varchar(128),
 interface varchar(128),
 penalty INT(11),
 PRIMARY KEY (queue_name, interface)

I do not have any strong preferences for either one, so in the end I choose the one I found was the simplest. Either model is easy to implement, so I am happy to be convinced otherwise.

I do not get your point that this should be automagic ... ? Yes storing queues.conf in static realtime is automatic. But then changes do not take effect until the next reload, whereas the whole point of this patch is that changes in the database take effect immediately without a reload.

By: Kevin P. Fleming (kpfleming) 2005-04-20 11:41:03

I think the two-table model is much preferable. It would be the first use of a multi-level structure in Asterisk realtime, but I think it's far more logical.

If you are willing to implement it this way, I'll take a look and get it committed as quickly as possible, because I know there are people that want this :-)

By: knielsen (knielsen) 2005-04-20 14:03:52

Kevin, I will implement the two-level structure (I kind of prefer it myself).

I should have a new patch ready sometimes next week.

I am thinking that I should use two database families (to specify the two
tables). This would make it possible to place the two tables under different
drivers, which is kind of strange but would actually work, I think.

Another annoyance is that with the current realtime framework I will need to
use two separate transactions to pull the queue data from two tables. So the
issue pops up what happens if changes happen between the two transactions. I
think things are mostly ok if I select from queue_table first and
queue_member_table second, though.

Or do you want me to add a new multi-table single-transaction method to the
realtime framework (I would be able to add it only for the MySQL driver)? I
think we should try to avoid that...

By: Brian West (bkw918) 2005-04-20 15:21:11

Well the way to do it is already there.. realtime will return an astconfig object filled out for you... no need to parse the var val pair crap outside the already existing framework.


By: Kevin P. Fleming (kpfleming) 2005-04-20 23:51:45

Except that doesn't work very well for multiple instances of the same variable name in a context, as is needed in queues.conf for "member =>" support.

Here's anohter thought, although it's more radical (but along the lines of the current discussion about redesigning voicemail.conf). What about something like this:


strategy = ...

interface = Agent/612
penalty = 2

interface = SIP/phone1

interface = IAX2/darkwingduck

I'm not sure really that the "member names" here would have any real value, although I can envision some use for them in the future.

This sort of configuration would work nicely with realtime as it never duplicates a variable name within a context, but it's a bit more wordy for non-realtime configs.


By: knielsen (knielsen) 2005-04-21 02:00:31

Kevin, what you suggest would not work for dynamic realtime (as was the point
of this patch), as far as I can see.

For dynamic realtime, we need a way for the database to quickly look up the
members of a queue, as we will be doing that each time a caller enters the
queue. With the [member@queue] syntax we would need to scan the whole config
every time.

What you suggest would probably work fine for static config (realtime or not),
but that issue is not affected by the patch. It just looks that way because
I moved some of the static config code into queue_set_param() so the code could
be shared.

I think for dynamic realtime we should stick with the two-table approach.

By: Kevin P. Fleming (kpfleming) 2005-04-21 11:24:57

Hmm, I see what you mean. I think I agree then, the two-table approach is the way to go, but it's possible we need to take what you create and make a new API for realtime that can collapse a two-table lookup into an ast_config object.

By: knielsen (knielsen) 2005-04-25 05:01:01

Ok, here is a new patch, updated to be against the newest CVS as of

Note that this patch replaces the previous patch. Only the patch
`rt-queue-patch2-dist.txt' needs to be applied for this feature.

This patch uses the two-table approach discussed above. Suitable MySQL
table definitions are the following:

   CREATE TABLE queue_table (
     name                   VARCHAR(128) PRIMARY KEY,
     musiconhold            VARCHAR(128),
     announce               VARCHAR(128),
     context                VARCHAR(128),
     timeout                INT(11),
     monitor_join           BOOL,
     monitor_format         VARCHAR(128),
     queue_youarenext       VARCHAR(128),
     queue_thereare         VARCHAR(128),
     queue_callswaiting     VARCHAR(128),
     queue_holdtime         VARCHAR(128),
     queue_minutes          VARCHAR(128),
     queue_seconds          VARCHAR(128),
     queue_lessthan         VARCHAR(128),
     queue_thankyou         VARCHAR(128),
     queue_reporthold       VARCHAR(128),
     announce_frequency     INT(11),
     announce_round_seconds INT(11),
     announce_holdtime      VARCHAR(128),
     retry                  INT(11),
     wrapuptime             INT(11),
     maxlen                 INT(11),
     servicelevel           INT(11),
     strategy               VARCHAR(128),
     joinempty              VARCHAR(128),
     leavewhenempty         VARCHAR(128),
     eventmemberstatus      BOOL,
     eventwhencalled        BOOL,
     reportholdtime         BOOL,
     memberdelay            INT(11),
     weight                 INT(11),
     timeoutrestart         BOOL

   CREATE TABLE queue_member_table (
     queue_name varchar(128),
     interface varchar(128),
     penalty INT(11),
     PRIMARY KEY (queue_name, interface)

The columns in the queue_table use the same names as parameters in the
config file (except that I allow the use of underscore `_' instead of
dash `-' to make life easier with SQL). All columns are optional with
the exception of the `name' column (in the queue table) and `queue_name'
and 'interface' (in the member table).

The extconfig.conf needs two families, for example

   queues => mysql,asterisk,queue_table
   queue_members => mysql,asterisk,queue_member_table

and the queues.conf needs to reference the two families:

   realtime_family = queues,queue_members

By: ckruetze (ckruetze) 2005-04-25 09:19:42

Just an idea, but what do you think of splitting it into a three table design?

CREATE TABLE queue_table (
     name VARCHAR(128) PRIMARY KEY,

CREATE TABLE queue_option (
     queue_name VARCHAR(128),
     option_name VARCHAR(128),
     option_value VARCHAR(128),
     PRIMARY KEY (queue_name, option_name)

In the two table design, I can see two problems. a) we have fields in the table that might never get used and b) if somebody comes up with a new feature, the table needs to be changed and backwards compatibility might be a problem.

The only problem I see might be that not all queue options are varchar(128) and that this would generate a slight overhead, but not as much as having lots of never used fields in the table.

If a good database design is wanted, then it might even be useful to split the queue_option into queue_option_varchar and queue_option_int and maybe queue_option_bool.
maybe queue_option_bool.

Just my idea as I'm more a database programmer then an * programmer.

By: Brian West (bkw918) 2005-04-26 17:51:49

Realtime already supports doing this:

say you have members: SIP/12;SIP/13;Zap/1;Zap/g1/5551212;Agent/1

It would parse that correctly and return the object filled out just like if you put them all on single lines.


By: Brian West (bkw918) 2005-04-26 17:52:34

Also with any modern sql server you can do this with a view thus not having to hand parse the crap with your backend scripts.


By: Kevin P. Fleming (kpfleming) 2005-04-26 21:47:55

bkw: How do you propose handling the per-member penalty setting with them combined into a single string?

The three-table approach is a non-starter, because it uses a different method of converting database entries into Realtime config objects and I see no reason to introduce that at this point. If you are concerned about space consumption, do the following:

1) Allow NULLs in your columns.
2) Don't use any fixed-length string columns.

With those two changes to the table schema, having a bunch of extra columns carries nearly no cost at all. Adding new columns in the future is also a piece of cake, and is already the way that Realtime works.

Code notes from a quick review:

- Can you confirm that the compiler will optimize out a call to strncpy when the source string is a constant of length shorter than the variable? If it won't, don't use strncpy to initialize all the queue_* sound file names, since we know the constant strings will not be too long for the variables in question.

- Minor typos: s/queue.conf/queues.conf/, s/queue-rounding-seconds/announce-round-seconds/ (actually, error messages about parameter values should just use the already-found parameter name, which will ease future changes/copying of the code)

- Don't compare pointers to NULL (m == NULL), just use !m instead

- In reload_queue_rt, you will call ast_strdupa every time you find a parameter with an '_' in it. This will eat up stack space very quickly, as they are not freed until the function returns. Just allocate a buffer on the stack, and keep reusing it as you need it.

- You can avoid the 'first_field' stuff when looping through member variables by using ast_load_realtime_multientry; look at app_directory for an example.

- I don't see a call to ast_variables_destroy for member_vars.

- While you are in there, please change join_queue to use something like:

if (!q) {
return res;

This way all the other code doesn't need to be indented inside a block, when there are only two lines outside of it.

- Is there some value in having the 'realtime_family' setting in queues.conf? I think it's safe to always call them 'queues' and 'queue_members', and always make the calls into ast_realtime. If the user hasn't configured them in extconfig.conf, they will be no-ops.

Otherwise, let me say this _loudly_: This is a darn fine piece of work! Major kudos to you for your first patch being this complex and of such high quality.

By: knielsen (knielsen) 2005-04-27 10:08:35

Kevin, thanks for your comments, well spotted for the two memory issues!

And thanks for the kind words :-).

I implemented all of your suggestions in the new patch
`rt-queue-patch3-dist.txt'. This is against the newest CVS as of today,
and replaces the previous two patches (so I suggest you delete patch2
just as you deleted patch1 earlier).

Anything else before this could go in CVS?

By: Kevin P. Fleming (kpfleming) 2005-04-27 11:51:36

I think is ready to go... it applies and compiles cleanly, and appears that it would work properly (I don't have a database configured here, but the code is straightforward in that regard).

I've marked this 'confirmed' so it's queued for Mark to approve.

By: Brian West (bkw918) 2005-04-27 13:25:04

Never mind.. I see it.. two tables is more sane than the var/val stuff... and does fit more in line with realtime ...


edited on: 04-27-05 13:25

edited on: 04-27-05 13:27

By: Kaj J. Niemi (kajtzu) 2005-04-27 15:18:54

When fetching rows from queue members, is the first part of the select statement ("... interface LIKE '%' AND ...") necessary? The table definition states that the interface column cannot be NULL. I'm just curious :)

Apr 27 22:44:08 DEBUG[9551] res_config_mysql.c: MySQL RealTime: Retrieve SQL: SELECT * FROM queues WHERE name = 'my-test'
Apr 27 22:44:08 DEBUG[9551] res_config_mysql.c: MySQL RealTime: Everything is fine.
Apr 27 22:44:08 DEBUG[9551] res_config_mysql.c: MySQL RealTime: Retrieve SQL: SELECT * FROM queue_members WHERE interface LIKE '%' AND queue_name = 'my-test'
ORDER BY interface
Apr 27 22:44:08 DEBUG[9551] res_config_mysql.c: MySQL RealTime: Everything is fine.

By: knielsen (knielsen) 2005-04-28 02:26:07

Yes, the "... interface LIKE '%' AND ..." is pretty wierd, and I do not
like it. But it is necessary because of the way realtime is designed.

Kevin asked me to use the ast_load_realtime_multientry() function to
fetch members, which makes a lot of sense since we are fetching multiple
rows. So we need to tell that function to fetch all rows that match a
given key `queue_name', and return the rows indexed by the column

Unfortunately the API for that function is seriously broken, IMHO, in
that you cannot explicitly specify the column `interface' on which you
want to index the rows in the returned ast_config structure. Instead the
index column is implicitly set from the first join condition passed to
the function. So I have to put in an initial no-op join condition on the
`interface' column. I choose "interface LIKE '%'", another option would
have been "interface != ''". Look at app_directory.c, it does the same

   rtdata = ast_load_realtime_multientry("voicemail", "mailbox LIKE", "%", "context", context, NULL);

I am quite tempted to go in and fix this (maybe just give
realtime_multi_mysql() and friends an extra argument specifying the
column to index on and appropriate changes in config.c). While there I
would also want to fix the "SQL injection" vulnerability that exists
currently because of the lack of any use of proper SQL quoting or bind
variable usage.

It's just that I want my first appearance in the Asterisk development
community to be a humble "here is a nice little feature, feel free to
use it in Asterisk", not a crashing "you framework is badly broken, you
need to change it like this _now_" :-).

In any case, this is a separate issue, and if there is a consensus that
ast_load_realtime_multientry() should be changed I will open a new bug
for it.

By: Kevin P. Fleming (kpfleming) 2005-04-28 11:18:09

This analysis is all spot-on; there are some definite design deficiencies in that API. However, fixing them is something that will need to happen in a separate bug, after this one has been merged.

By: knielsen (knielsen) 2005-05-12 13:42:05

Any progress with this?

By: Kevin P. Fleming (kpfleming) 2005-05-14 22:51:53

Mark: I think this is ready to go, but need your approval since it's a significant addition.

By: Mark Spencer (markster) 2005-05-15 18:59:29

I definitely like this patch.  I'd like some other people to sign off on the functionality test, code review and security audit, but conceptually I'm totally on board with this architecture and am ready to see it merged pending those reviews.

By: Kevin P. Fleming (kpfleming) 2005-05-15 19:41:34

The patch from bug 3821 has made this one no longer apply cleanly, so it will need updating. While that's being done, you can replace the strncpy() calls with ast_copy_string() calls instead :-)

When the new patch is posted I'm ready to sign off on the code/security/formatting reviews, as long as there are no regressions from the current patch. I've posted a request to the asterisk-dev list for testers so we can get some additional functionality testing done as well.

By: drmac (drmac) 2005-05-15 20:12:51

I will test this patch in a production environment tomorrow.

From what I've read about this patch, this patch is step 1 of 3 to making asterisk completly load balancable across multiple servers sharing 1 database.  app_queue has been keeping me from starting up a server farm load balanced by SER (or some other layer 7 switch) because its 'not' db enabled. kudos to nielsen for this.

By: Kevin P. Fleming (kpfleming) 2005-05-16 09:52:30

I already reported that in the bugnote above yours; did you review them?

By: drmac (drmac) 2005-05-16 11:11:56

crap...(post deleted)

By: outtolunc (outtolunc) 2005-05-16 14:36:47

uploaded http://www.dynx.net/ASTERISK/diff-patches/app_queue.c.diff.txt
it has not been tested but it compiles cleanly.  it was a 'by-hand' edit of the rt-queue-patch3-dist.txt against current cvs-head as of noon 05-16-2005 (PST)

disclaimer on file (though this is not my code <G>)

By: drmac (drmac) 2005-05-16 19:23:22

patch applied successfully.

I just tried to add myself to a queue listed in database using AddQueueMember.
Debug log shows no Realtime activity; therefor I get "queue not found" error.

(does some investigating..)

ok. i think i see. you can't do login/logoff of agents. all agents are static. hmm..but we can change that easily enough by adding an "active" column to the queue_memebers table. thus when an agent logs in/logs off, the "active" column is ast_realtime_update'd.

?? ideas on that ??

here is my vision of this patch:
X number of asterisk servers all sharing a common database. company ABC's queue is stored in database. agents for ABC login/logoff throughout the day.
all X asterisk servers are load balanced. any incomming call could goto any server.
agent 1 logs in at server A. database updated. agent 2 at B, agent 3 at C.
person calls in and gets added to queue on server B. server B says "which agents are available?" query db. "oh. agent 1 is available. contact/bridge this call to him."
server B knows how to contact agent 1 because agent 1 registered using realtime SIP; so we can create a full contact header.

?? thoughts on that ??

By: knielsen (knielsen) 2005-05-17 07:23:03

I had some issues with outtolunc's app_queue.c.diff.txt patch, so I merged it
into current CVS myself and will upload as rt-queue-patch4-dist.txt.

The only difference between this and the previous patch is the change of
strncpy() to ast_copy_string(). So there were acually no conflicts with
bug 3821, only with the ast_copy_string() updates :-(

By: Terry Wilson (twilson) 2005-05-17 16:31:58

Works fine for me so far (using postres w/ odbc)

By: knielsen (knielsen) 2005-05-18 03:42:10

Just a quick comment about AddQueueMember. The AddQueueMember stuff uses the embedded Berkley DB in Asterisk, not realtime. This patch doesn't try to change that, and frankly I think that should be kept as a separate issue. Once this patch is in, a new bug can be opened on getting the stuff working that the comment from drmac mentions.

By: Kevin P. Fleming (kpfleming) 2005-05-19 00:36:29

As I understand it, there are three types of queue members supported by app_queue:

- static members
- dynamic members (AddQueueMember)
- agents (static members of the type Agent/<name> handled by chan_agent)

Static members do not allow any sort of login/logout functionality. Dynamic members do not either, but they can be added/removed as needed. Agents do support login/logout, of two different varieties (callback and normal).

This patch only addresses static members and agents, and does not affect the login/logout behavior of the agents since that is handled by chan_agent.

In spite of that, I think we _do_ need to allow for AddQueueMember to be called against a queue that is loaded via Realtime, since we support it for queues from queues.conf. I agree that there is no need to be able to persist them to the Realtime database at this time, they can continue to be persisted to the astdb as they are now. A future patch could extend that behavior, of course, as already commented.

Comments? Do we need to hold this patch until Realtime queues can receive dynamic members, or merge it as it stands and hope that functionality comes along soon :-)

By: knielsen (knielsen) 2005-05-19 03:49:35

Ok, I looked deeper into the AddQueueMember issue (never used that myself).

For a realtime queue, AddQueueMember works the same as an INSERT into the member table, and a RemoveQueueMember works the same as a DELETE from the member table.

So to properly implement AddQueueMember/RemoveQueueMember is simple enough. In add_to_queue()/remove_from_queue(), use reload_queue_rt() to load the queue. If the queue is static do the stuff in the current code. If the queue is realtime, instead issue the proper INSERT or DELETE statement to the database (and maybe update the in-memory structure afterwards to get the change to take effect also for callers currently sitting in the queue). That should be sufficient.

Only problem: the realtime framework does not support INSERT or DELETE (at least not the res_config_mysql.c one) :-(

Kevin, I think you had in mind that the AddQueueMember stuff would load the queue from realtime and then add extra dynamic members to the in-core linked lists for the queue. But that would create a synchronisation nightmare between realtime members and dynamic members in the same queue, and I cannot really see anyone wanting this functionality (certainly it's not what drmac needs). And if noone wants it, who would implement it?

drmac, you wrote the res_mysql_config.c, would you be willing to add INSERT/DELETE to that driver (and to config.c) and to test a realtime-persistent AddQueueMember as outlined above?

BTW, I will not be able to do any work on this patch in June or July.

By: Terry Wilson (twilson) 2005-05-19 10:23:44

Well, as long as we are coming up with a way that we think it should be...

Lets look at queues and queue members from the point of designing an interface for them (since they are realtime, a gui is a prime example of a good use for them).  It would be nice to be able to assign members to a queue, and have them be able to log in and out.  If we do it the current way for realtime that is currently done w/ AddQueueMember/RemoveQueueMember then we would be physically deleting the members so there would be no record of having them haaving existed after they logged out.  Having a flag for active/not active would make it easy to define the members via realtime, and the flip the flag for loggged in or not.  So as far as asterisk is concerned, the query could contain WHERE active = 1 for it to recognize the member, but you could still have them defined for the interface.

Personally I use res_perl so I can add my own flag and own add_queue_member remove_queue_member functions that do this (as long as I can add the WHERE active = 1 clause to the SQL in the realtime queue code)... but just thought I'd give my two cents.

Admittedly this is probably outside the scope of the original patch.  I don't think that we have to wait to get the dynamic stuff in to get this submitted.  We can discuss all of those considerations in a future bug.

By: outtolunc (outtolunc) 2005-05-19 11:03:27

i'm not sure if this is the right time or place to mention this.  but as long as you guys are heading this direction.  

if you look at the relationships..    

dbget / dbput
database get / database put
realtime load / update

what is wrong with that picture..  we need a a realtime put (insert)

just my $.02

By: drmac (drmac) 2005-05-19 11:16:01

Re: post ASTERISK-2718651

I have no problem implementing a ast_realtime_insert and ast_realtime_delete in res_config_mysql.c. I actually have both of them already coded for my personal use. The reason I never added them publicly was because res_config_odbc did not have them.

If we don't want to do insert/delete, we can always add another column to the queuemember table called "active" and just use ast_realtime_update to change that column from 1 to 0 and vice versa depending on Add/Remove.

The caveat with that is you have to pre-load all members into the table which isn't the current behavior.

By: Michael Jerris (mikej) 2005-05-21 09:42:37

markster, can we have some comment on realtime insert\delete?

By: Michael Jerris (mikej) 2005-05-21 09:45:34

Also lets get this in and move implementing realtime updates and delete to another bug.

By: knielsen (knielsen) 2005-05-31 03:41:49

I think we should finish this issue one way or the other, add to CVS or not, but let's finish it.

I looked once more into the AddQueueMember stuff, and I still think the only sane way is to map them to realtime INSERT/DELETE, once that is added to realtime. The comment from outtolunc seems to agree.

Or perhaps use drmac's realtime UPDATE proposal, but that also requires realtime additions: the current realtime API does not allow to update on a composite key, and realtime queue need the key (queue, member).

Both twilson and MikeJ seemed to think this should be added to CVS and the dynamic stuff moved to another bug.

Or if you prefer, I will just have to maintain this locally for our own purpose.

By: Kevin P. Fleming (kpfleming) 2005-06-02 17:39:59

Committed to CVS HEAD, thanks!

By: Digium Subversion (svnbot) 2008-01-15 15:36:51.000-0600

Repository: asterisk
Revision: 5821

U   trunk/apps/app_queue.c
U   trunk/configs/extconfig.conf.sample

r5821 | kpfleming | 2008-01-15 15:36:50 -0600 (Tue, 15 Jan 2008) | 2 lines

add realtime support to app_queue for static members/agents (bug ASTERISK-3943)