[asterisk-dev] [Code Review] 4318: res_pjsip_pubsub: Fix persistent subscriptions not surviving graceful shutdown

[George Joseph]

> On Jan. 7, 2015, 8:52 a.m., Mark Michelson wrote:
> > 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.
> 
> George Joseph wrote:
>     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.
> 
> Mark Michelson wrote:
>     Yeah, ast_shutting_down()'s existence in channel.[hc] is a bit odd considering it has nothing to do with channels.
>     
>     "... you absolutely can't send a NOTIFY/terminated to a subscriber and expect to pick up where you left off."
>     Correct. If you terminate the subscriptions, you should not expect to be able to pick up where you left off. My thoughts are that if you are shutting down Asterisk permanently (or even just long-term), then you want the phones/endpoints to have their subscriptions shut down so the subscriptions can be restarted when a subscription server is once again available.
>     
>     "Also, testing for restart vs shutdown isn't viable as you may perform an asterisk shutdown to reboot or solve an issue."
>     I think I may not have been as clear as I could have been here. My suggestion was that on restarts, always persist subscriptions. On a shutdown, make it optional whether the subscriptions persist. The idea is you have an option somewhere (though frankly, I'm not sure where this would be defined) that says "on a shutdown, do not close open subscriptions", presumably because persisting them makes sense since Asterisk will soon be coming back up. On a restart, we would ignore this option and always persist the subscriptions because obviously, Asterisk is about to start right back up again. That's why I suggested the ability to discern between a restart and a shutdown.
>     
>     "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."
>     There are several reasons for this presumption not to hold up (for instance, Olle's suggestion of DNS SRV).
>     To me, passing the buck to subscribers doesn't seem fair. If we have the ability to cleanly shut down subscriptions when Asterisk shuts down, we should do that if it is desired. If shutting down subscriptions is not desired, then we should also cater to that use case.

"My thoughts are that if you are shutting down Asterisk permanently (or even just long-term), then you want the phones/endpoints to have their subscriptions shut down so the subscriptions can be restarted when a subscription server is once again available."
True.  I guess the problem is that if you tell a phone to terminate a subscription, will it ever restart without a kick in the butt?  The Digium and Grandstream phones don't.

"The idea is you have an option somewhere (though frankly, I'm not sure where this would be defined) that says "on a shutdown, do not close open subscriptions""
Ah, gotcha.  How about pjsip.conf [global]?

"There are several reasons for this presumption not to hold up"
If the option works, then I'm fine.


- George


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4318/#review14104
-----------------------------------------------------------


On Jan. 7, 2015, 8:24 a.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4318/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2015, 8:24 a.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   branches/13/res/res_pjsip_pubsub.c 430293 
>   branches/13/main/asterisk.c 430293 
>   branches/13/include/asterisk/options.h 430293 
> 
> Diff: https://reviewboard.asterisk.org/r/4318/diff/
> 
> 
> Testing
> -------
> 
> 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.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150107/bf1a7ec0/attachment.html>