Consider adding a bosh property to customize TerminateTimeout

[youngm]

https://github.com/cloudfoundry/executor/blob/0c06103dc4fa8cc5fc98c4dc7302752d6351f9b2/depot/steps/run_step.go#L18

Would allow deployers the ability to control the time between sigterm and sigkill. Some customers workloads are more sensitive to restarts and although allowing the customer to set this value on a per application basis might be the optimal solution to this problem, allowing the deployer to customize it for the platform would be a good step in the direction of better understanding need and potential problems around different terminate timeout values.

Thanks.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/151464387

The labels on this github issue will be updated when the story is started.

[youngm]

As an optional bonus it would be nice if some logging were added so we could monitor applications that have instances killed via sigkill. There is a debug message today, I think an info message to that effect with app identifiable information would be very helpful.

[emalm]

Hey, @youngm, thanks for starting this discussion on Slack a while ago. I've just added https://www.pivotaltracker.com/story/show/152431251 for the Diego team to prioritize soon; could you let me know if you have any feedback about it?

Thanks,
Eric

[youngm]

Thanks for keeping this issue going @emalm I think the story looks great.

A couple of side questions:

• Does the route-emitter currently send the removal of the app instance to the routers prior to the shutdown cycle?
• When draining a cell Is the current shutdown process one application at a time serially or concurrent for all or many of the applications on the cell?

[youngm]

@emalm Nice work with this issue! I think you delivered exactly what I'm looking for. We'll get this value into our next release and start gathering some useful data.

[emalm]

Hey, thanks, @youngm ! We'd really appreciate some feedback on it from you and your devs.

Oh, I still owe you answers to your questions above -- I'll try to do that soon, and certainly before we close out this issue.

Thanks again,
Eric

[emalm]

Regarding your earlier questions, @youngm:

• When an app is stopped or scaled down (a destructive change in desired state for the app), the route-emitter will react to the change in desired state by sending unregistration messages for those instances at the same time that the rep signals those instances to shut down gracefully. We generally observe that those unregistration messages propagate to the edge routers within milliseconds, in comparison to the seconds-long graceful shutdown interval.
• Similarly, when a cell is evacuating and a replacement LRP instance starts successfully on another cell, the BBS removes the evacuating record for the old instance, and the route-emitter reacts to its removal by unregistering the route to that instance. The evacuating cell then also reacts to the removal of that evacuating record by stopping the old instance gracefully.
• We have identified a case in which restarting an individual app instance via cf restart-app-instance doesn't cause its route to be unregistered, and we're assessing what we would need or want to do about that case (see placeholder story at https://www.pivotaltracker.com/story/show/153437108). Actually, if you have app developers or app or platform operators who currently use that command, we'd be interested to know why: its original intent was to allow app developers to restart an individual app instance that had gotten into a bad state from which it would not recover but was not causing it to terminate the app process, and in that case it has seemed less important to cut off requests for that instance gracefully.
• When a cell evacuates its instances, replacement instances are all submitted for scheduling simultaneously. As those replacements transition to running successfully on other cells, the evacuating cell stops the corresponding old instance gracefully.

Hope that helps! I'll close this issue out, but as always I'm happy to continue discussion on GitHub or in Slack.

Thanks,
Eric

[emalm]

Oh, for posterity, we delivered this configurability in Diego v1.32.0.

[youngm]

That was great! Thank you @emalm . I do have users who regularly use "restart-app-instance" as a way to do a kind of rolling restart for various reasons. Once we have a real rolling restart solution then I suspect we won't use this feature for that reason anymore. Nobody has noticed the current issue with restart-app-instance so from my perspective I would give it a low priority and instead look towards rolling restart. Hope that helps!

Mike