> On 2012-04-19 08:12:24, fpj wrote: > > One quick clarification. Is the reason why you want to have a separate file > > for membership to separate membership from the other configuration > > parameters? If so, by having this change in, we are saying that other > > parameters must be static. > > Alexander Shraer wrote: > Excellent point. I'm not totally sure, but it seems that some parameters > will have to be statically configured on each server, such as the dataDir. > Others are dynamic. At first stage, only the membership is dynamic but other > parameters like tickTime syncLimit may become dynamic in the future. However, > you are totally right in that the dynamic parameters are not limited to > membership in general and I therefore changed all references to this in the > patch. For example, the extension of an automatically created dynamic > configuration file is now ".dynamic" instead of ".members" and the pointer to > that file from the normal config file is now "dynamicConfigFile=..." instead > of "memberhsipFile=..."
Sounds better to me, thanks for fixing it. > On 2012-04-19 08:12:24, fpj wrote: > > /src/java/test/org/apache/zookeeper/server/util/MembershipBCTest.java, line > > 45 > > <https://reviews.apache.org/r/4729/diff/1/?file=101924#file101924line45> > > > > Clarification. This is the only new test, so I'm assuming that forward > > compatibility is being tested through the other tests that are modified to > > work with the membership file. > > Alexander Shraer wrote: > yep Got it, thanks. > On 2012-04-19 08:12:24, fpj wrote: > > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 306 > > <https://reviews.apache.org/r/4729/diff/1/?file=101914#file101914line306> > > > > Does it need to be public? There are a number of public of methods > > following this one and I'm not sure they should be. > > Alexander Shraer wrote: > these methods are used in QuorumPeerMain.java and tests similarly to > other methods, such as setMinSessionTimeout(), and similarly they were maid > public. QuorumPeerMain is in the same package. For tests, you may want to extend QuorumPeer so that you have access to non-public methods. - fpj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4729/#review7021 ----------------------------------------------------------- On 2012-04-19 16:01:55, Alexander Shraer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4729/ > ----------------------------------------------------------- > > (Updated 2012-04-19 16:01:55) > > > Review request for zookeeper. > > > Summary > ------- > > https://issues.apache.org/jira/browse/ZOOKEEPER-1411 > > Currently every server has a different configuration file. With this patch, > we will have all cluster membership definitions in a single file, and every > sever can have a copy of this file. > > > Diffs > ----- > > /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java > 1327937 > /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1327937 > /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java > 1327937 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java > 1327937 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1327937 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java > 1327937 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > 1327937 > > /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java > 1327937 > > /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java > 1327937 > /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java > 1327937 > > /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java > 1327937 > /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java > 1327937 > /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java > 1327937 > /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1327937 > /src/java/test/org/apache/zookeeper/server/util/DynamicConfigBCTest.java > PRE-CREATION > /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1327937 > /src/java/test/org/apache/zookeeper/test/FLETest.java 1327937 > /src/java/test/org/apache/zookeeper/test/FLEZeroWeightTest.java 1327937 > /src/java/test/org/apache/zookeeper/test/HierarchicalQuorumTest.java > 1327937 > /src/java/test/org/apache/zookeeper/test/LENonTerminateTest.java 1327937 > /src/java/test/org/apache/zookeeper/test/ObserverTest.java 1327937 > /src/java/test/org/apache/zookeeper/test/QuorumBase.java 1327937 > /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1327937 > /src/java/test/org/apache/zookeeper/test/StandaloneTest.java 1327937 > > Diff: https://reviews.apache.org/r/4729/diff > > > Testing > ------- > > Many tests were updated to work with the new configuration format. > MembershipBCTest.java tests backward compatibility. > > > Thanks, > > Alexander > >
