On Thu, Jun 22, 2017 at 11:20 PM, Alexander Shraer <[email protected]> wrote:
> I'm not sure it's necessary for backward compatibility since rolling > restarts for config changes are not really an api the system provides. > Yeah, it is not part of API in a strict sense but there are tools / management software build on top of rolling restarts. Also the principal of least surprise to users - especially given that we did not explicitly mention in the doc that rolling restarts is deprecated on 3.5.x (the doc only mentioned rolling restart is 'not needed' anymore :). It seems reasonable to provide an option to at least give users sometime to migrate to dynamic reconfig while they can continue use the rolling restarts. > > I'd think the ACL control and admin only API should be sufficient for > security and would prefer to get rid of the flag. But if you must have it, > we have to prevent both in memory config updates (most important) and > config file updates if reconfig is disabled. This sounds like a small > change in quorumpeer, but perhaps I'm forgetting something. > Filed https://issues.apache.org/jira/browse/ZOOKEEPER-2819 for this. > > Cheers > Alex > > > On Thu, Jun 22, 2017 at 11:06 PM Michael Han <[email protected]> wrote: > > > Hi Alex, thanks for clarification! > > > > It makes sense to me that users should use reconfig instead of rolling > > upgrade moving forward. The only concern is backward compatibility now we > > drop the old rolling upgrade support: since 3.5.x needs to be backward > > compatible with 3.4.x [1], I think we probably need support rolling > upgrade > > for 3.5 branch. > > > > As for this flag - I believe it's there and set to false because reconfig > > is a security sensitive feature and for such features user has to opt in > > explicitly. Our security team here also has similar recommendations when > I > > talked with them about what this feature could do. There are also some > > discussions around this flag / why it's there in ZOOKEEPER-2014. > > > > [1] > > https://cwiki.apache.org/confluence/display/ZOOKEEPER/ReleaseManagement > > > > > > On Thu, Jun 22, 2017 at 10:39 PM, Alexander Shraer <[email protected]> > > wrote: > > > > > Hi Michael, > > > > > > The described behavior is the intended one - in 3.5 configuration is > part > > > of the synced state and is updated > > > when the server syncs with the leader. The only rolling upgrade I > tested > > > was to upgrade the software version > > > of the servers - this should still work. But I didn't try to support > > > rolling upgrade for upgrading the configuration, > > > since this should be done through reconfig. > > > > > > I'm still not sure what's the purpose of this flag btw. Why would > someone > > > want to do rolling restarts which are prone > > > to inconsistencies and data loss, when they can use reconfig ? > > > > > > Alex > > > > > > > > > > > > > > > On Thu, Jun 22, 2017 at 10:18 PM, Michael Han <[email protected]> > wrote: > > > > > > > reconfigEnabled only disables reconfig command when > > > reconfigEnabled=false; > > > > it does not disable the feature by mute all code paths of the > reconfig > > > > feature introduced in ZOOKEEPER-107. So regardless of the value of > > > > reconfigEnabled, > > > > 3.5.x ZK will create static config file and dynamic config file in > any > > > > cases. > > > > > > > > This might create a problem for users who want to do rolling upgrade > > the > > > > old way - because now the critical config information is not stored > in > > > > zoo.cfg anymore and modifying cfg.dynamic file manually will not work > > > > because a reconfig needs to go through quorum processors. I think > this > > is > > > > the problem described in the thread. > > > > > > > > Alex, is reconfig compatible with rolling upgrade? I don't find > > anything > > > > mentioned in ZOOKEEPER-107 about this. Currently I think the answer > is > > > no, > > > > which means for 3.5.x the only way to change membership of cluster is > > > > through reconfig. Could you confirm this conclusion? If that is the > > case > > > we > > > > need patch the reconfigEnabled so it completely disable all code path > > of > > > > the reconfig feature to leave the static zoo.cfg intact. > > > > > > > > > > > > On Thu, Jun 22, 2017 at 9:35 PM, Alexander Shraer <[email protected] > > > > > > wrote: > > > > > > > > > This sounds like a bug in the implementation of reconfigEnabled. > > > > > Could you please open a JIRA with the description you provided ? > > > > > > > > > > Out of curiosity, why do you disable reconfig ? It is intended > > exactly > > > > > to perform the changes you're trying to make, in a simple and > correct > > > > way. > > > > > > > > > > Thanks, > > > > > Alex > > > > > > > > > > On Thu, Jun 22, 2017 at 3:17 PM, Guillermo Vega-Toro < > > > > [email protected]> > > > > > wrote: > > > > > > > > > > > I'm still unable to make configuration changes when > > > > reconfigEnabled=false > > > > > > by updating zoo.cfg and restarting the servers. > > > > > > > > > > > > For example, I want to change the weight of one of my servers. I > > edit > > > > > > zoo.cfg on the server I want to change, and specify the group, > > > > server.x, > > > > > > and weight.x properties for all servers. I also remove the > > > > > > dynamicConfigFile property and delete the dynamic config file. I > > then > > > > > > restart the server. As soon as the server starts, the dynamic > > config > > > > file > > > > > > re-appears, and it has the last configuration, as if the changes > I > > > made > > > > > in > > > > > > zoo.cfg were ignored. The dynamic configuration file on the other > > > > servers > > > > > > also stays the same. > > > > > > > > > > > > What would be the correct way to achieve this (change a server's > > > > weight, > > > > > > or role) when reconfigEnabled=false and the CLI reconfig command > > > cannot > > > > > be > > > > > > used? > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Cheers > > > > Michael. > > > > > > > > > > > > > > > -- > > Cheers > > Michael. > > > -- Cheers Michael.
