> On Aug. 9, 2014, 1:49 p.m., Alexander Shraer wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1340 > > <https://reviews.apache.org/r/24208/diff/1-10/?file=649201#file649201line1340> > > > > Looking at this change again it seems that this can potentially change > > the behavior of tests that previously started without a static config file > > but have a dynamic file. Before the change they would continue writing out > > the dynamic file when the configuration changed. After the change this > > whole block of code is not run , so no dynamic file changes would happen. > > > > One issue is that you're always basing the dynamic filename on the > > static filename but in this case you don't have the static one. Perhaps you > > should have dynamicConfigFilenamePrefix or something like that ? > > > > so instead of using configFilename + ".dynamic" + number maybe you > > should do dynamicConfigFilenamePrefix + number if > > dynamicConfigFilenamePrefix is not null. > > > > See condition of the "if" statement before the change. Perhaps you can > > propose a better solution. > > > > > > Alexander Shraer wrote: > on second thought it may be ok as it is - the test that I have in mind is > ReconfigTest.java, but this test is not testing recovery, so its probably > fine if it doesn't write configuration stuff on disk.
So instead of making more a decisive change, I would make a getDynamicFilenamePrefix(), which still takes configFilename + ".dynamic" for this moment. How did that go for you? - Hongchao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24208/#review50111 ----------------------------------------------------------- On Aug. 8, 2014, 2:54 p.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24208/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2014, 2:54 p.m.) > > > Review request for zookeeper. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-1994 > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 76f0afc > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java > c4397a1 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > 0a8a45a > src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java > 89416ca > src/java/test/org/apache/zookeeper/server/quorum/ReconfigBackupTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/server/quorum/ReconfigLegacyTest.java > c05aa1b > src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java > 1a090dc > > Diff: https://reviews.apache.org/r/24208/diff/ > > > Testing > ------- > > > Thanks, > > Hongchao Deng > >
