| Summary: | ASTERISK-12504: [patch] Calls in high-weighted queue block low-weighted | ||
| Reporter: | garychen (garychen) | Labels: | |
| Date Opened: | 2008-08-01 08:28:01 | Date Closed: | 2010-05-18 10:44:14 |
| Priority: | Major | Regression? | No |
| Status: | Closed/Complete | Components: | Applications/app_queue |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) app_queue.c ( 1) my_patch.txt ( 2) my_patch1.txt | |
| Description: | I have two queues, weighted 4 and 5. High priority calls are placed in Q6002, while low priority ones in Q6001. All agents are member of both queues. The problem is whenever there's a high priority call ringing on an agent (the agent not answer the call), low priority calls not routed to any idle agents. They are blocked. ****** ADDITIONAL INFORMATION ****** [general] persistentmembers=yes autofill=yes wrapuptime=2 shared_lastcall=yes [6001] wrapuptime=1 strategy=rrmemory reportholdtime=no joinempty=loose leavewhenempty=loose eventwhencalled=yes retry=1 maxlen=0 weight=4 [6002] wrapuptime=0 strategy=rrmemory reportholdtime=no joinempty=loose leavewhenempty=loose eventwhencalled=yes retry=1 maxlen=0 weight=5 | ||
| Comments: | By: Tilghman Lesher (tilghman) 2008-08-05 10:00:50 That makes perfect sense to me. You're essentially guaranteeing that the high priority call will get serviced first, even if there are low priority calls sitting in queue. What is the problem? By: garychen (garychen) 2008-08-05 10:14:08 The only high priority call is already ringing (get serviced?), but the agent does not pick it up. There're still other queue members ready and idle, the low priority call should be routed at this moment. The problem is, other agents only see it after the ringing high priority call be answered. By: Tilghman Lesher (tilghman) 2008-08-05 10:30:33 No, it is already using the correct behavior. The issue is that a high priority call needs to be answered first before lower priority callers may be queued. If you want autofill behavior, that can be turned on with autofill=yes, but that's not the way it is configured. By: garychen (garychen) 2008-08-05 10:49:15 autofill=yes is already turned on. It doesn't make sense, let's say, you have 10 agents there, 1 urgent call comes to 1 agent who does not answer it immediately, makeing all other 9 agents not able to see any regular calls. PS, I have 2 queues, urgent calls and regular calls are in different queues, regular call is the head in it's own queue. 10 agents are members of both queues. By: garychen (garychen) 2008-09-10 12:08:56 I fixed my problem by adding a condition on line 1738: //if ( q->weight > rq->weight) { if ( (q->weight > rq->weight) && ( q->count >= get_available_members(q) ) ) { The function get_available_members() is to calculate the number of agents that are not in use, which means not in talking or ringing. It's logic is pretty much the same as is_our_turn(). It would be better to merge together to avoid duplication. By: Tilghman Lesher (tilghman) 2008-11-07 15:55:05.000-0600 garychen: If you want to sign a license agreement and upload your patch, I believe it would be accepted. By: garychen (garychen) 2008-11-10 09:05:50.000-0600 Corydon76: I am not able to sign the agreement because there is no captcha image shown on that page. On FireFox, it shows nothing. On IE, it shows a red-cross of bad image. By: Tilghman Lesher (tilghman) 2008-11-10 11:42:39.000-0600 garychen: the captcha has been fixed. By: Leif Madsen (lmadsen) 2009-02-02 14:56:27.000-0600 garychen: I suppose you couldn't upload this patch as a unified diff could you? either diff -u against the vanilla source, or: svn diff | tee /tmp/my_patch.txt By: Leif Madsen (lmadsen) 2009-02-02 14:56:59.000-0600 Assigned to putnopvut for when the patch is uploaded, or maybe he knows how to fix it simply and can just close the issue out. Thanks! By: Leif Madsen (lmadsen) 2009-02-27 16:41:16.000-0600 Changing status to Confirmed as there is a patch attached. By: Mark Michelson (mmichelson) 2009-03-03 15:55:48.000-0600 garychen, could you please upload your patch as a unified diff instead of uploading the entirety of app_queue.c? It makes it much easier to see what actual changes you have made. By: garychen (garychen) 2009-03-03 16:55:52.000-0600 I checked out latest version of 1.4 and discovered that there's lots of changes, so maybe my version is too old. anyway, I uploaded 2 files: my_patch.txt is my working copy against latest my_patch1.txt is the change for this issue By: Mark Michelson (mmichelson) 2009-03-18 10:51:57 I've started a branch to fix this issue. It can be checked out here: http://svn.digium.com/svn/asterisk/team/mmichelson/issue13220 Seeing your patch has helped me to understand the bug better and gives a great starting point for trying to get this solved. By: Mark Michelson (mmichelson) 2009-03-18 17:25:51 I've finished the necessary work for this to be merged. I have placed a review request on review board. The URL for the review request is here: http://reviewboard.digium.com/r/202/ By: Digium Subversion (svnbot) 2009-03-30 11:17:36 Repository: asterisk Revision: 185031 U branches/1.4/apps/app_queue.c ------------------------------------------------------------------------ r185031 | mmichelson | 2009-03-30 11:17:35 -0500 (Mon, 30 Mar 2009) | 39 lines Fix queue weight behavior so that calls in low-weight queues are not inappropriately blocked. (This is copied and pasted from the review request I made for this patch) 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. (closes issue ASTERISK-12504) Reported by: garychen Review: http://reviewboard.digium.com/r/202/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=185031 By: Digium Subversion (svnbot) 2009-03-30 11:26:49 Repository: asterisk Revision: 185072 _U trunk/ U trunk/apps/app_queue.c ------------------------------------------------------------------------ r185072 | mmichelson | 2009-03-30 11:26:48 -0500 (Mon, 30 Mar 2009) | 45 lines Merged revisions 185031 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r185031 | mmichelson | 2009-03-30 11:17:35 -0500 (Mon, 30 Mar 2009) | 39 lines Fix queue weight behavior so that calls in low-weight queues are not inappropriately blocked. (This is copied and pasted from the review request I made for this patch) 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. (closes issue ASTERISK-12504) Reported by: garychen Review: http://reviewboard.digium.com/r/202/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=185072 By: Digium Subversion (svnbot) 2009-03-30 11:40:27 Repository: asterisk Revision: 185087 _U branches/1.6.0/ U branches/1.6.0/apps/app_queue.c ------------------------------------------------------------------------ r185087 | mmichelson | 2009-03-30 11:40:27 -0500 (Mon, 30 Mar 2009) | 52 lines Merged revisions 185072 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r185072 | mmichelson | 2009-03-30 11:26:48 -0500 (Mon, 30 Mar 2009) | 45 lines Merged revisions 185031 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r185031 | mmichelson | 2009-03-30 11:17:35 -0500 (Mon, 30 Mar 2009) | 39 lines Fix queue weight behavior so that calls in low-weight queues are not inappropriately blocked. (This is copied and pasted from the review request I made for this patch) 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. (closes issue ASTERISK-12504) Reported by: garychen Review: http://reviewboard.digium.com/r/202/ ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=185087 By: Digium Subversion (svnbot) 2009-03-30 11:47:22 Repository: asterisk Revision: 185088 _U branches/1.6.1/ U branches/1.6.1/apps/app_queue.c ------------------------------------------------------------------------ r185088 | mmichelson | 2009-03-30 11:47:22 -0500 (Mon, 30 Mar 2009) | 52 lines Merged revisions 185072 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r185072 | mmichelson | 2009-03-30 11:26:48 -0500 (Mon, 30 Mar 2009) | 45 lines Merged revisions 185031 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r185031 | mmichelson | 2009-03-30 11:17:35 -0500 (Mon, 30 Mar 2009) | 39 lines Fix queue weight behavior so that calls in low-weight queues are not inappropriately blocked. (This is copied and pasted from the review request I made for this patch) 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. (closes issue ASTERISK-12504) Reported by: garychen Review: http://reviewboard.digium.com/r/202/ ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=185088 By: Digium Subversion (svnbot) 2009-03-30 11:52:30 Repository: asterisk Revision: 185089 _U branches/1.6.2/ U branches/1.6.2/apps/app_queue.c ------------------------------------------------------------------------ r185089 | mmichelson | 2009-03-30 11:52:29 -0500 (Mon, 30 Mar 2009) | 52 lines Merged revisions 185072 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r185072 | mmichelson | 2009-03-30 11:26:48 -0500 (Mon, 30 Mar 2009) | 45 lines Merged revisions 185031 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r185031 | mmichelson | 2009-03-30 11:17:35 -0500 (Mon, 30 Mar 2009) | 39 lines Fix queue weight behavior so that calls in low-weight queues are not inappropriately blocked. (This is copied and pasted from the review request I made for this patch) 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. (closes issue ASTERISK-12504) Reported by: garychen Review: http://reviewboard.digium.com/r/202/ ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=185089 | ||