<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1535/">https://reviewboard.asterisk.org/r/1535/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 20th, 2011, 10:47 a.m., <b>Matthew Nicholson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">There appears to be some sort of race condition between when a device state is updated and when we try to ring a device. I don't understand how all this works well enough to know for sure, but even with this change it still seems possible that the device state could change between when we check here and when we actualy dial. It is impossible to get this 100% perfect, as we can't control who may be dialing the device outside of asterisk, but I suppose we could make this work right within asterisk. Either way, that is outside of the limited scope of this change.
This change looks good. In my performance testing it has a much much lesser effect on performance than the previous code that was here. There are still some channel list walks involved, but they don't appear to be causing massive performance degradation like the previous change did.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think you mean 'outside of app_queue' here, not 'outside of Asterisk'. Your analysis is correct, though; there is no 'atomic' method to test device status and then dial, nor is there any way to 'reserve' a device so that it won't be dialed through any other means until you've decided whether you want to dial it or not. There are ways such problems could be solved, but they would require some significant new APIs... and it seems that they are possibly past the point of diminishing returns, because the frequency that a call will be delivered to an otherwise busy device with the proposed changes in place should be quite small.</pre>
<br />
<p>- Kevin</p>
<br />
<p>On October 20th, 2011, 5:49 a.m., irroot wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and Matthew Nicholson.</div>
<div>By irroot.</div>
<p style="color: grey;"><i>Updated Oct. 20, 2011, 5:49 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">commit r325483 introduced a performance problem reverted by r341486
here is a proposal to do this more "friendly" its a global option [possibly should be
per Q] and uses ast_device_state that tries to get the state from a channel driver
it calls ast_parse_device_state only if this fails to yield a result it then fires a
devicestate event to set it so it does not get called over and over.
the reality is its happening 4 out of 1000 times this check is run that the agent is busy
this is a minimal % the customer who raised this issue is under strict SLA to answer calls
and provide service they a medical aid call center who deal with e911 cases authorizing
treatment.
hope this rework is more acceptable ill be putting this in production and reporting results back.
in the log files [rotated daily] from 15/09/2011 to 19/10/2011
110507 times was logged
516 duplicate calls prevented
ill be watching these metrics closely in next bit.
the problem is its not efficient at all as they only had 46598 calls come into the system.
-------------
The regression was caused by a call to ast_parse_device_state() in app_queue's
ring_entry() function. The ast_parse_device_state() function eventually calls
ast_channel_get_full() with a channel name prefix which causes it to walk the
channel list causing massive lock contention and slow downs.
This patch fixes the regression by removing the call to
ast_parase_device_state() which should be unnecessary. Queue member device
state should be maintained by device state events. Some users have seen
instances where busy agents were called when they shouldn't have, which is the
reason the call to ast_parse_device_state() was added. That change appears to
have resolved that issue but also causes this performance regression. There may
still be issues with queue member status, and if so, alternative methods should
be investigated to resolve them.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/AST-695">AST-695</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/10/apps/app_queue.c <span style="color: grey">(341525)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1535/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>