[asterisk-dev] [Code Review] Fix queue weight behavior so that calls in low-weighted queues are not blocked when they should not be.

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/202/#review642
-----------------------------------------------------------

Ship it!


Looks good to me, nice work!

- Russell


On 2009-03-18 16:43:43, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/202/
> -----------------------------------------------------------
> 
> (Updated 2009-03-18 16:43:43)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Asterisk has some odd behavior when queue weights are used. The current logic used when potentially calling a queue member is:
> 
> If the member we are going to call is part of another queue and _that other queue has any callers in it_ and has a higher weight than the queue we are calling from, then don't try to contact that member. The issue here is what I have marked with underscores. If the higher-weighted queue has any callers in it at all, then the queue member will be unreachable from the lower-weighted queue. This has the potential to be really really bad if using a queue strategy, such as leastrecent or fewestcalls, with the potential to call the same member repeatedly.
> 
> The fix proposed by garychen on issue 13220 is very simple and, as far as I can see, works well for this situation. With this set of changes, the logic used becomes:
> 
> If the member we are going to call is part of another queue, the other queue has a higher weight than the queue we are calling from, and the higher weight queue has at least as many callers as available members, then do not try to contact the queue member. If the higher weighted queue has fewer callers than available members, then there is no reason to deny the call to this member since the other queue can afford to spare a member.
> 
> Since the fix involved writing a generic function for determining the number of available members in the queue, I also modified the is_our_turn function to make use of the new num_available_members function to determine if it is our turn to try calling a member. There is one small behavior change. Before writing this patch, if you had autofill disabled, then if you were the head caller in a queue, you would automatically be told that it was your turn to try calling a member. This did not take into account whether there were actually any queue members available to take the call. Now we actually make sure there is at least one member available to take the call if autofill is disabled.
> 
> 
> This addresses bug 13220.
>     http://bugs.digium.com/view.php?id=13220
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/apps/app_queue.c 183027 
> 
> Diff: http://reviewboard.digium.com/r/202/diff
> 
> 
> Testing
> -------
> 
> I first of all tested to make sure that simple calls to a queue would properly report the number of available members still. That passed just fine.
> 
> I also tested to be sure that the weight behavior has been modified as desired, and it worked well, too.
> 
> 
> Thanks,
> 
> Mark
> 
>