<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/4318/">https://reviewboard.asterisk.org/r/4318/</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 7th, 2015, 8:52 a.m. MST, <b>Mark Michelson</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 option flag you've introduced is unnecessary. There is an ast_shutting_down() function that you can call to determine if Asterisk is shutting down.
I've been thinking about this a bit more since yesterday, and I think the behaviour you're introducing isn't always going to be wanted on a shutdown. It may be that this instance of Asterisk is being shut down permanently in order to migrate to a new server, or something similar. In such a case, keeping the subscriptions open during shutdown doesn't really make sense. But if the system is going down temporarily, say for a brief maintenance, on a shutdown it might make sense to persist the open subscriptions. I think making this behaviour optional on shutdown is a good idea.
During a restart, I imagine that this introduced behavior will always be wanted, though. The problem is, there's no way to tell if the current shutdown operation is part of a restart. This may be an area where introducing a new option flag is warranted, to tell if Asterisk is being restarted. This way, you can always persist the subscriptions during a restart.</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;">Funny, I saw ast_shutting_down in channel.c but ignored it because I thought it was channel specific.
I'm open to other suggestions but you absolutely can't send a NOTIFY/terminated to a subscriber and expect to pick up where you left off. They'll respond with a 481. Also, testing for restart vs shutdown isn't viable as you may perform an asterisk shutdown to reboot or solve an issue. I think it's up to the subscribers to handle the cases of a permanent shutdown. Presumably they're going to have to be reconfigured anyway to pick up a new server.</pre>
<br />
<p>- George</p>
<br />
<p>On January 7th, 2015, 8:24 a.m. MST, George Joseph 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 and Mark Michelson.</div>
<div>By George Joseph.</div>
<p style="color: grey;"><i>Updated Jan. 7, 2015, 8:24 a.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;">If you do a 'core (shutdown|restart) graceful' persistent subscriptions won't survive. If you do a 'core (shutdown|restart) now' or asterisk terminates for some reason, they do. Here's why...
When asterisk shuts down gracefully, it sends a 'NOTIFY/terminated' to subscribers for each subscription. This not only tells the subscribers that the dialog/state machine is done, it also frees the last reference to the subscription tree which causes the persistent subscription to get deleted from astdb. When asterisk restarts, nothing's left. Just preventing the delete from astdb doesn't work because we already told the subscriber to terminate the dialog so we can't restart it even if it was still in astdb. Everything works OK if asterisk terminates unexpectedly because we never send the 'terminated' message so on restart, the subscription is still in astdb and the subscriber is none the wiser.
This patch suppresses the sending of 'NOTIFY/terminated' on shutdown for persistent connections. To do this, I've had to add a new AST_OPT_FLAG_SHUTTING DOWN to options so we know that a shut down is in progress.</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;">Lots of before and after Wireshark testing with Digium, Grandstream, Unidata and soft phones. Now when asterisk comes back up, it sends NOTIFY/active messages with updated expires to persistent subscribers which all return OK and further notifies are processed correctly.</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/13/res/res_pjsip_pubsub.c <span style="color: grey">(430293)</span></li>
<li>branches/13/main/asterisk.c <span style="color: grey">(430293)</span></li>
<li>branches/13/include/asterisk/options.h <span style="color: grey">(430293)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4318/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>