----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19459/#review37888 -----------------------------------------------------------
./src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java <https://reviews.apache.org/r/19459/#comment69665> 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. - Edward Ribeiro On March 20, 2014, 10:58 a.m., Rakesh R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19459/ > ----------------------------------------------------------- > > (Updated March 20, 2014, 10:58 a.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 1579527 > ./src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java > 1579527 > ./src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java > 1579527 > ./src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java > 1579527 > ./src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1579527 > > Diff: https://reviews.apache.org/r/19459/diff/ > > > Testing > ------- > > standalone tests included, quorum test case to be added. > > > Thanks, > > Rakesh R > >
