<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/2996/">https://reviewboard.asterisk.org/r/2996/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 7th, 2013, 3:37 p.m. UTC, <b>Joshua Colp</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;">To make sure I better understand the situation...
Is it caching an old stale entry without address?
Then when device state is queried it uses that old stale entry instead of requerying realtime... so it thinks it has no address when in realtime there really is an address?</pre>
</blockquote>
<p>On November 7th, 2013, 3:39 p.m. UTC, <b>Joshua Colp</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;">Well, no, cause your title says uncached... I'm more confused about how it's returning a peer because devicestate purposely doesn't check realtime.</pre>
</blockquote>
<p>On November 8th, 2013, 11:19 a.m. UTC, <b>wdoekes</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;">Thanks for the review. We are indeed talking about uncached:
rtcachefriends=no
The relevant code:
sip_devicestate(...)
sip_find_peer(host, NULL, realtime=FALSE, FINDALLDEVICES, devstate_only=TRUE, 0)
if (!p && (realtime || devstate_only)) {
p = realtime_peer(peer, addr, devstate_only, which_objects);
realtime_peer(...)
if (ast_test_flag(&global_flags[1], SIP_PAGE2_RTCACHEFRIENDS) && !devstate_only) {
...
ao2_t_link(peers, peer, "link peer into peers table");
So, the devstate_only=TRUE initiates a realtime lookup anyway, but ensures that the found peer isn't linked in the peers table (if rtcachefriends were to be true).
The comments in sip_devicestate could probably use some clarification.
As for your question:
> Is it caching an old stale entry without address?
No, it's returning the freshly loaded peer, but it will never have an p->addr (unless it is currently in a call); the proxy takes care of that stuff. So I need the devicestate to return what it knows, which is *nothing* (unknown), not *unavailable*.</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;">Proxy? An outbound proxy?</pre>
<br />
<p>- Joshua</p>
<br />
<p>On November 5th, 2013, 2:41 p.m. UTC, wdoekes wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By wdoekes.</div>
<p style="color: grey;"><i>Updated Nov. 5, 2013, 2:41 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<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;">When using realtime SIP peers as queue members, the device state as returned by sip_devicestate() is AST_DEVICE_UNAVAILABLE when the peer is not loaded into memory.
For uncached realtime SIP peers, this is practically always. That means that my queue doesn't have any available members.
This patch changes all p->realtime peers with a null p->addr to return AST_DEVICE_UNKNOWN instead. If we set the queue to accept unknown members, the queue correctly copes with devices with this UNKNOWN state:
static int member_status_available(int status)
{
return status == AST_DEVICE_NOT_INUSE || status == AST_DEVICE_UNKNOWN;
}
A different approach would have been to set the p->defaddr to non-null, but that would put the device in NOT_INUSE state which is not entirely correct, since I don't know if the device *is* available. And I haven't checked what other side-effects setting the p->defaddr has.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Tested on asterisk 10 and 11 with the SIP-devices in the state_interface column. Without the patch, the device state is unavailable, and the queue will report the members as unavailable. With the patch, the members have the unknown state and things work properly. </pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/channels/chan_sip.c <span style="color: grey">(402467)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2996/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>