evertrue / zookeeper-cookbook

Chef cookbook for installing and managing Zookeeper.
https://supermarket.chef.io/cookbooks/zookeeper
Apache License 2.0
81 stars 119 forks source link

Notify service to restart on config change #174

Closed ErebusBat closed 8 years ago

jeffbyrnes commented 8 years ago

@ErebusBat this is, generally speaking, a bad idea. If enough of your ZK nodes restarted to lose quorum, you would lose state.

Additionally, if ZK is being managed by Exhibitor or a similar service, you want to let that software handle the restarts instead.

The advice, generally, is to manually handle restarts, or use Exhibitor to manage them for you. Chef isn’t a great candidate for this sort of thing.

That said, if you had an attribute that controlled if a config change notified to restart, which defaulted to false, I’d be willing to consider that functionality.

williamsjj commented 8 years ago

I second this being a bad idea for the reasons stated...I would argue even having the option is dangerous. Newbies will turn it on, and there's a high likelihood someone will inadvertently merge a change that enables it.

ErebusBat commented 8 years ago

@jeffbyrnes All good points. Unfortunately not everyone is, or can, run exhibitor.

However @williamsjj does bring some very good points to the table. The config_render recipe is very simple and can be easily extracted by those who want/need this functionality.

jeffbyrnes commented 8 years ago

@ErebusBat very true. Thanks again for the contribution, even if it is one that is too dangerous.