|Summary:||ASTERISK-02877: [patch] Reload dynamic queue members on restart|
|Date Opened:||2004-11-23 17:13:45.000-0600||Date Closed:||2008-01-15 15:15:45.000-0600|
|Environment:||Attachments:||( 0) reload_queue_members-app_queue.c.diff.txt|
( 1) reload_queue_members-app_queue.c.diff.txt
( 2) reload_queue_members-app_queue.c.patch
( 3) reload_queue_members-app_queue.c.patch.txt
( 4) reload_queue_members-app_queue.c.diff.txt
( 5) reload_queue_members-app_queue.c.diff.txt
( 6) reload_queue_members-queues.conf.sample.diff.txt
|Description:||When asterisk is restarted, all queues are reloaded which inherently removes all dynamically added queue members. As this is potentially dangerious as calls may be missed if an agent forgets to or is not properly informed that they must relogin to the queue. It would be a nice feature to have dynamic members in each queue be automatically added after a restart.|
I've attached a patch for app_queue.c which will record, to a file (currently /var/lib/asterisk/queue_members.db) each member in each queue. When the load_module() function is called, each member in the file will be readded back into each specific queue.
****** ADDITIONAL INFORMATION ******
I have also added a configuration option which can turn this feature off. It must be located in the [general] group in queues.conf. I have set this feature to default on. To turn it off configure queues.conf as such:
persistent_members = 0
I have also faxed a disclaimer.
|Comments:||By: Brian West (bkw918) 2004-11-23 17:24:32.000-0600|
Wouldn't it be better to store this information in the astdb like we do in chan_sip for seeding?
By: kevinl (kevinl) 2004-11-23 17:32:00.000-0600
Yes, that probably sounds better. I will take a look into that. thanks.
By: Brian West (bkw918) 2004-11-23 17:36:52.000-0600
Also if you use the astdb it keeps state if your box crashes.
By: Mark Spencer (markster) 2004-11-23 23:14:10.000-0600
DB is already atomic, so it should be relatively straight forward to store entries there.
By: kevinl (kevinl) 2004-11-24 13:56:52.000-0600
The latest uploaded patch now uses astdb, thanks bkw918.
By: Brian West (bkw918) 2004-11-24 14:11:18.000-0600
Is a malloc really needed here?
Here is what I would do (and did in cdr_odbc.c)
char value = "";
snprintf(value, sizeof(value),"%s/%s;%s;", cur_member->tech, cur_member->loc, penalty_str);
By: kevinl (kevinl) 2004-11-24 15:05:08.000-0600
no, a malloc isn't really needed there. Putting it on the stack would be best. For some reason I was thinking I was using 4096k but it is really 4k and I didn't want to fill up the stack.
I will fix. Should I keep reuploading the patches? I also forgot to add credits so...
By: kevinl (kevinl) 2004-11-25 12:09:25.000-0600
Fixed up the malloc, and the possible overrun which slipped past me. Let me know if you see anything else.
By: Brian West (bkw918) 2004-11-25 12:36:27.000-0600
#define PM_MAX_LEN 2048
memset(value, 0, sizeof(value));
Those small tweaks... 4096 is a bit large.. 2048 should be plenty.
edited on: 11-25-04 12:39
By: kevinl (kevinl) 2004-11-29 16:43:34.000-0600
Changes done. Let me know if you need anything else.
By: Brian West (bkw918) 2004-11-29 20:40:34.000-0600
I vote for this to be on all the time with no option to turn it off. We do this in chan_sip.
By: Mark Spencer (markster) 2004-12-01 00:19:49.000-0600
This is not the correct usage of strncat (man strncat) to prevent a buffer overrun. I suggest: snprintf(value + strlen(value), sizeof(value) - strlen(tmp_value), ... ). We also might think about how to most gracefully handle a failure.
I do like the idea that this is an option (should be off by default) because it can incur a performance penalty. Also we need to see a patch for the config file to show how the new option is used.
This is a great contribution and I'm eager to get it included!
By: kevinl (kevinl) 2004-12-01 12:33:00.000-0600
ack, thanks Mark. I'll fix that strncat. The single snprintf suggestion works for me, although I think you meant: ?
snprintf(value + strlen(value), sizeof(value) - strlen(value), ... ).
The performance hit should be quite low as the code is only run when an agent is added/removed from a queue, and on module start, which is generally not very often.
The only issue I see with keeping it default to on is if asterisk is started after it being down for a long period of time and having dynamic agents automatically added to the queues when they are not desired.
As for failing gracefully, if an error occures the result will either be a partial record of agents logged into each queue stored in the astdb, or upon startup, only some of the recorded members being added back into their queues. Errors are reported to help show if anything goes wrong. At least this is how I tried to design it.
By: Kevin P. Fleming (kpfleming) 2004-12-05 16:04:54.000-0600
There is one more possible problem with doing this (and it's not a problem with your code, it's a problem with the whole idea, even though I support it <G>).
If Asterisk is restarted while SIP/foo.phone is a dynamic queue member, it will be re-added to the queue, even if foo.phone is longer a valid SIP peer for this Asterisk instance (if it's been removed from the SIP peer list in sip.conf or realtime database). While this is not a terribly bad problem, it does mean that the queue can have no "real" members, but callers will still be placed into it because this one member is there, even though calls cannot be delivered to the member. This particular peer also cannot remove itself from the queue, because it is longer a valid peer (well, at least on a basic level, obviously there are ways to do it in spite of the configuration change).
Is there a way that this startup process could do some sort of "validation" call into the relevant channel technology driver to see if it can still accept that member before re-adding the member to the queue? This is not fool-proof either, but it would certainly help.
By: Mark Spencer (markster) 2004-12-06 00:56:26.000-0600
Added to CVS with some structural modifications and changing the option to "persistentmembers" rather than "persistent_members" and making the default "off" so as to retain current behavior for existing installations and making "yes" the default in the sample config for new installs. Thank you very much for your contribution!
By: Russell Bryant (russell) 2004-12-06 18:10:04.000-0600
not in 1.0
By: Digium Subversion (svnbot) 2008-01-15 15:15:45.000-0600
r4390 | markster | 2008-01-15 15:15:45 -0600 (Tue, 15 Jan 2008) | 2 lines
Add persistent dynamic queue member support (bug ASTERISK-2877)