> On March 20, 2014, 2:17 p.m., Edward Ribeiro wrote: > > ./src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java, > > line 217 > > <https://reviews.apache.org/r/19459/diff/2/?file=529349#file529349line217> > > > > I would rewrite lines 217-242 as two methods: > > public void unlock() throws IOException { > > boolean exceptionOccured = releaseLock(dataDirLock, "data") || > > releaseLock(snapDirLock, "snap"); > > if (exceptionOccured) { > > throw new IOException("Failed to release ZooKeeper dir > > lock!"); > > } > > } > > > > private boolean releaseLock(FileLock lock, String dirName) throws > > IOException { > > try { > > if (lock != null) { > > lock.release(); > > lock.channel().close(); > > } > > return false; > > } catch (IOException ioe) { > > LOG.error(String.format("Failed to release the %s directory > > lock!", dirName), ioe); > > return true; > > } > > finally { > > lock = null; // it's okay to unreference even in case of > > failure? > > } > > } > > > > There's not much LoC savings, only a minor code repetition avoidance, > > so feel free to ignore this suggestion. > >
pls see the latest patch, where I've done the changes. - Rakesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19459/#review37888 ----------------------------------------------------------- On March 20, 2014, 6:31 p.m., Rakesh R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19459/ > ----------------------------------------------------------- > > (Updated March 20, 2014, 6:31 p.m.) > > > Review request for zookeeper, fpj, michim, Raul Gutierrez Segales, and > Camille Fournier. > > > Bugs: ZOOKEEPER-1502 > https://issues.apache.org/jira/browse/ZOOKEEPER-1502 > > > Repository: zookeeper > > > Description > ------- > > Prevent multiple ZooKeeper servers are using the same data directories at > same time. > Here trying to address the case where one ZK server JVM is running, what if > another server mistakenly configured the the same data dir and starts. > > > Diffs > ----- > > ./src/java/main/org/apache/zookeeper/server/ZKDatabase.java 1579683 > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java > 1579683 > ./src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java > 1579683 > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java > 1579683 > ./src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1579683 > ./src/java/test/org/apache/zookeeper/test/QuorumTest.java 1579683 > > Diff: https://reviews.apache.org/r/19459/diff/ > > > Testing > ------- > > standalone tests included, quorum test case to be added. > > > Thanks, > > Rakesh R > >
