<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/1652/">https://reviewboard.asterisk.org/r/1652/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 10th, 2012, 8:44 a.m., <b>Matt Jordan</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;">The issue I have with this patch as it currently exists is that it changes expected behavior in 1.8 unilaterally, for something that is arguably not a bug (at least for a small to medium number of peers). There may be people who, because they reload chan_sip often, depend on the qualify going out immediately. However, I can see the case for this going into 1.8 as, for a large number of peers, the behavior may be unacceptable.
I'd prefer to see a global option in sip.conf that would control this behavior. The options would be to:
(a) schedule the qualify immediately on reload using the previous logic, and use the range of times to schedule in the case of initial startup (keeping the qualifyfreq as the maximum range however, which - in my opinion - does make more sense the hardcoded 5 seconds)
(b) use the range of times for all cases
Default option would be (a), but would allow people with a large number of defined peers to throttle back the pokes.
Either way, changes in the behavior of chan_sip should also be noted in the CHANGES file.</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;">Noting the change in the CHANGES is good, but adding more options feels like overkill to me.
Can't we have it "guess" whether we want behaviour (a) or (b):
e.g. count(peers) >= 100, use (b)</pre>
<br />
<p>- wdoekes</p>
<br />
<p>On January 5th, 2012, 3:14 a.m., schmidts 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.</div>
<div>By schmidts.</div>
<p style="color: grey;"><i>Updated Jan. 5, 2012, 3:14 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;">with a big amount of sip peers registered doing a sip reload causes a packet flood of sip OPTIONS messages cause they are immediately sent out when the peer is loaded back from the astdb.
This fix prevents this packet storm and just schedule a poke in a random value between 1 ms and peers qualifyfreq or the global qualifyfreq.
Another very small fix is to schedule a poke on peers with qualify disabled. This doesnt cost much but its just unnecessary to do this.
maybe it would be even better to dont call the poke function from source_db on a reload cause poking all peers is done after this again.</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 with 4500 peers with qualify=no and no poke is scheduled after a sip reload.
tested with 2500 peers with qualify=yes and pokes are scheduled in the right time (1 to 60000 ms)</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/ASTERISK-19154">ASTERISK-19154</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/1.8/channels/chan_sip.c <span style="color: grey">(349449)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1652/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>