[asterisk-commits] mmichelson: branch mmichelson/queue-penalty r93826 - in /team/mmichelson/queu...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Dec 18 18:05:10 CST 2007


Author: mmichelson
Date: Tue Dec 18 18:05:09 2007
New Revision: 93826

URL: http://svn.digium.com/view/asterisk?view=rev&rev=93826
Log:
Queue penaltychange rules will not blow up on reload now.

This was done by creating a copy of the queue's rules list for each caller.
This implementation carries a limitation; a caller will only follow the rules
that were defined for the queue when the caller entered the queue. If the rules
change for the queue while the caller is waiting, these new rule changes will not
be reflected on that caller. This is documented in queue.conf.sample.

Other changes made in this commit include changing the lists to not have locks. Instead
we use the queue's lock when we change the queue's list. For traversal of the caller's copy
of the list, no lock is necessary since each caller is handled in his own thread. Also, since
we don't have a lock, there's no need to explicitly call AST_LIST_HEAD_INIT.

Next step is to test what's here and see if I come across any bugs. If none are found,
it's on to enhancements!


Modified:
    team/mmichelson/queue-penalty/apps/app_queue.c
    team/mmichelson/queue-penalty/configs/queues.conf.sample

Modified: team/mmichelson/queue-penalty/apps/app_queue.c
URL: http://svn.digium.com/view/asterisk/team/mmichelson/queue-penalty/apps/app_queue.c?view=diff&rev=93826&r1=93825&r2=93826
==============================================================================
--- team/mmichelson/queue-penalty/apps/app_queue.c (original)
+++ team/mmichelson/queue-penalty/apps/app_queue.c Tue Dec 18 18:05:09 2007
@@ -314,29 +314,30 @@
 
 
 struct queue_ent {
-	struct call_queue *parent;          /*!< What queue is our parent */
-	char moh[80];                       /*!< Name of musiconhold to be used */
-	char announce[80];                  /*!< Announcement to play for member when call is answered */
-	char context[AST_MAX_CONTEXT];      /*!< Context when user exits queue */
-	char digits[AST_MAX_EXTENSION];     /*!< Digits entered while in queue */
-	int valid_digits;		    /*!< Digits entered correspond to valid extension. Exited */
-	int pos;                            /*!< Where we are in the queue */
-	int prio;                           /*!< Our priority */
-	int last_pos_said;                  /*!< Last position we told the user */
-	time_t last_periodic_announce_time; /*!< The last time we played a periodic announcement */
-	int last_periodic_announce_sound;   /*!< The last periodic announcement we made */
-	time_t last_pos;                    /*!< Last time we told the user their position */
-	int opos;                           /*!< Where we started in the queue */
-	int handled;                        /*!< Whether our call was handled */
-	int pending;                        /*!< Non-zero if we are attempting to call a member */
-	int max_penalty;                    /*!< Limit the members that can take this call to this penalty or lower */
-	int linpos;							/*!< If using linear strategy, what position are we at? */
-	int linwrapped;						/*!< Is the linpos wrapped? */
-	time_t start;                       /*!< When we started holding */
-	time_t expire;                      /*!< When this entry should expire (time out of queue) */
-	struct ast_channel *chan;           /*!< Our channel */
-	struct penalty_rule *pr;			/*!< The penalty rule we will implement next */
-	struct queue_ent *next;             /*!< The next queue entry */
+	struct call_queue *parent;             /*!< What queue is our parent */
+	char moh[80];                          /*!< Name of musiconhold to be used */
+	char announce[80];                     /*!< Announcement to play for member when call is answered */
+	char context[AST_MAX_CONTEXT];         /*!< Context when user exits queue */
+	char digits[AST_MAX_EXTENSION];        /*!< Digits entered while in queue */
+	int valid_digits;		               /*!< Digits entered correspond to valid extension. Exited */
+	int pos;                               /*!< Where we are in the queue */
+	int prio;                              /*!< Our priority */
+	int last_pos_said;                     /*!< Last position we told the user */
+	time_t last_periodic_announce_time;    /*!< The last time we played a periodic announcement */
+	int last_periodic_announce_sound;      /*!< The last periodic announcement we made */
+	time_t last_pos;                       /*!< Last time we told the user their position */
+	int opos;                              /*!< Where we started in the queue */
+	int handled;                           /*!< Whether our call was handled */
+	int pending;                           /*!< Non-zero if we are attempting to call a member */
+	int max_penalty;                       /*!< Limit the members that can take this call to this penalty or lower */
+	int linpos;							   /*!< If using linear strategy, what position are we at? */
+	int linwrapped;						   /*!< Is the linpos wrapped? */
+	time_t start;                          /*!< When we started holding */
+	time_t expire;                         /*!< When this entry should expire (time out of queue) */
+	struct ast_channel *chan;              /*!< Our channel */
+	AST_LIST_HEAD_NOLOCK(,penalty_rule) qe_rules; /*!< Local copy of the queue's penalty rules */
+	struct penalty_rule *pr;               /*!< Pointer to the next penalty rule to implement */
+	struct queue_ent *next;                /*!< The next queue entry */
 };
 
 struct member {
@@ -464,7 +465,7 @@
 	int membercount;
 	struct queue_ent *head;             /*!< Head of the list of callers */
 	AST_LIST_ENTRY(call_queue) list;    /*!< Next call queue */
-	AST_LIST_HEAD(, penalty_rule) rules; /*!< The list of penalty rules to invoke */
+	AST_LIST_HEAD_NOLOCK(, penalty_rule) rules; /*!< The list of penalty rules to invoke */
 };
 
 
@@ -844,6 +845,7 @@
 static void init_queue(struct call_queue *q)
 {
 	int i;
+	struct penalty_rule *pr_iter;
 
 	q->dead = 0;
 	q->retry = DEFAULT_RETRY;
@@ -891,7 +893,8 @@
 			ast_str_set(&q->sound_periodicannounce[i], 0, "%s", "");
 	}
 
-	AST_LIST_HEAD_INIT(&q->rules);
+	while((pr_iter = AST_LIST_REMOVE_HEAD(&q->rules,list)))
+		ast_free(pr_iter);
 }
 
 static void clear_queue(struct call_queue *q)
@@ -1025,7 +1028,7 @@
 	rule->value = atoi(changestr); 
 
 	/*We have the rule made, now we need to insert it where it belongs*/
-	AST_LIST_LOCK(&q->rules);
+	ao2_lock(q);
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&q->rules, iter, list) {
 		if (rule->time < iter->time) {
 			AST_LIST_INSERT_BEFORE_CURRENT(rule, list);
@@ -1038,7 +1041,7 @@
 	if (!inserted) {
 		AST_LIST_INSERT_TAIL(&q->rules, rule, list);
 	}
-	AST_LIST_UNLOCK(&q->rules);
+	ao2_unlock(q);
 
 	return 0;
 }
@@ -1779,6 +1782,7 @@
 {
 	struct call_queue *q;
 	struct queue_ent *cur, *prev = NULL;
+	struct penalty_rule *pr_iter;
 	int pos = 0;
 
 	if (!(q = qe->parent))
@@ -1801,6 +1805,9 @@
 				prev->next = cur->next;
 			else
 				q->head = cur->next;
+			/* Free penalty rules */
+			while((pr_iter = AST_LIST_REMOVE_HEAD(&qe->qe_rules, list)))
+				ast_free(pr_iter);
 		} else {
 			/* Renumber the people after us in the queue based on a new count */
 			cur->pos = ++pos;
@@ -2628,12 +2635,10 @@
 			int max_penalty = qe->pr->relative ? qe->max_penalty + qe->pr->value : qe->pr->value;
 			char max_penalty_str[12]; /*XXX Will change this idiom and probably optimize things once everything is running */
 			snprintf(max_penalty_str, sizeof(max_penalty_str) - 1, "%d", max_penalty);
-			AST_LIST_LOCK(&qe->parent->rules);
 			pbx_builtin_setvar_helper(qe->chan, "QUEUE_MAX_PENALTY", max_penalty_str);
 			qe->max_penalty = max_penalty;
 			ast_log(LOG_DEBUG, "Set max penalty for caller %s to %d\n", qe->chan->name, qe->max_penalty);
 			qe->pr = AST_LIST_NEXT(qe->pr, list);
-			AST_LIST_UNLOCK(&qe->parent->rules);
 		}
 
 		/* Wait a second before checking again */
@@ -4027,6 +4032,20 @@
 	return 0;
 }
 
+static void copy_rules(struct queue_ent *qe)
+{
+	struct penalty_rule *pr_iter;
+	ao2_lock(qe->parent);
+	AST_LIST_TRAVERSE(&qe->parent->rules, pr_iter, list) {
+		struct penalty_rule *new_pr = ast_calloc(1, sizeof(*new_pr));
+		new_pr->time = pr_iter->time;
+		new_pr->value = pr_iter->value;
+		new_pr->relative = pr_iter->relative;
+		AST_LIST_INSERT_TAIL(&qe->qe_rules, new_pr, list);
+	}
+	ao2_unlock(qe->parent);
+}
+
 static int queue_exec(struct ast_channel *chan, void *data)
 {
 	int res=-1;
@@ -4122,7 +4141,8 @@
 	if (!join_queue(args.queuename, &qe, &reason)) {
 		ast_queue_log(args.queuename, chan->uniqueid, "NONE", "ENTERQUEUE", "%s|%s", S_OR(args.url, ""),
 			S_OR(chan->cid.cid_num, ""));
-		qe.pr = AST_LIST_FIRST(&qe.parent->rules);
+		copy_rules(&qe);
+		qe.pr = AST_LIST_FIRST(&qe.qe_rules);
 check_turns:
 		if (ringing) {
 			ast_indicate(chan, AST_CONTROL_RINGING);
@@ -4225,12 +4245,10 @@
 				char max_penalty_str[12]; /*XXX Will change this idiom and probably optimize things once everything is running */
 				max_penalty = qe.pr->relative ? qe.max_penalty + qe.pr->value : qe.pr->value;
 				snprintf(max_penalty_str, sizeof(max_penalty_str) - 1, "%d", max_penalty);
-				AST_LIST_LOCK(&qe.parent->rules);
 				pbx_builtin_setvar_helper(qe.chan, "QUEUE_MAX_PENALTY", max_penalty_str);
 				qe.max_penalty = max_penalty;
 				ast_log(LOG_DEBUG, "Changed the max penalty for caller %s to %d\n", qe.chan->name, qe.max_penalty);
 				qe.pr = AST_LIST_NEXT(qe.pr, list);
-				AST_LIST_UNLOCK(&qe.parent->rules);
 			}
 
 			/* If using dynamic realtime members, we should regenerate the member list for this queue */

Modified: team/mmichelson/queue-penalty/configs/queues.conf.sample
URL: http://svn.digium.com/view/asterisk/team/mmichelson/queue-penalty/configs/queues.conf.sample?view=diff&rev=93826&r1=93825&r2=93826
==============================================================================
--- team/mmichelson/queue-penalty/configs/queues.conf.sample (original)
+++ team/mmichelson/queue-penalty/configs/queues.conf.sample Tue Dec 18 18:05:09 2007
@@ -367,7 +367,10 @@
 ; a call to be opened to more members. The advantage to changing members this way
 ; as opposed to inserting the caller into a different queue with more members or reinserting
 ; the caller into the same queue with a different QUEUE_MAX_PENALTY set is that
-; the caller does not lose his place in the queue.
+; the caller does not lose his place in the queue. Note: There is a limitation to these
+; rules; a caller will follow the penaltychange rules for the queue that were defined
+; at the time the caller entered the queue. If an update to the rules is made during the
+; the caller's stay in the queue, these will not be reflected for that caller.
 ;
 ; The syntax for these rules is
 ; penaltychange => <number of seconds into the call>,<absolute or relative change to QUEUE_MAX_PENALTY>




More information about the asterisk-commits mailing list