Repository: zookeeper
Updated Branches:
  refs/heads/master aefb13f2b -> bebe4162b


ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at 
startup

ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at 
startup

This PR adds a check to protect ZK against configuring dataDir and dataLogDir 
opposingly.

When FileTxnSnapLog is created, it checks if transaction log directory contains 
snapshot files or vice versa, snapshot directory contains transaction log 
files. If so, the check throws LogdirContentCheckException or 
SnapdirContentCheckException, respectively, which translates to 
DatadirException at ZK startup in QuorumPeerMain and ZooKeeperServerMain.

If the two directories are the same, then no check is done.

For testing, I've added 4 new unit tests which cover the following cases:

1. transaction log and snapshot directories are different and they are used 
correctly (no Exception)
2. transaction log and snapshot directories are the same (in this case no check 
is done)
3. transaction log and snapshot directories are different and transaction log 
directory contains snapshot files (LogdirContentCheckException -> ZK quits)
4. transaction log and snapshot directories are different and snapshot 
directory contains transaction log files (SnapdirContentCheckException -> ZK 
quits)

Author: Mark Fenes <mfe...@cloudera.com>

Reviewers: Andor Molnár <an...@cloudera.com>, Abraham Fine <af...@apache.org>

Closes #450 from mfenes/ZOOKEEPER-2967 and squashes the following commits:

3f2ce609a [Mark Fenes] Re-run checks
3dba7f14a [Mark Fenes] ZOOKEEPER-2967: Add check to validate dataDir and 
dataLogDir parameters at startup
50e7150b3 [Mark Fenes] Trigger notification
5f8ada87f [Mark Fenes] ZOOKEEPER-2967: Add check to validate dataDir and 
dataLogDir parameters at startup
36fc5bd46 [Mark Fenes] ZOOKEEPER-2967: Add check to validate dataDir and 
dataLogDir parameters at startup
eb76730cb [Mark Fenes] ZOOKEEPER-2967: Add check to validate dataDir and 
dataLogDir parameters at startup
81c026fe8 [Mark Fenes] ZOOKEEPER-2967: Add check to validate dataDir and 
dataLogDir parameters at startup


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/bebe4162
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/bebe4162
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/bebe4162

Branch: refs/heads/master
Commit: bebe4162bc915b15b4fd90df9446bdf9f83ef3a8
Parents: aefb13f
Author: Mark Fenes <mfe...@cloudera.com>
Authored: Wed Feb 14 12:23:30 2018 -0800
Committer: Abraham Fine <af...@apache.org>
Committed: Wed Feb 14 12:23:30 2018 -0800

----------------------------------------------------------------------
 .../zookeeper/server/persistence/FileSnap.java  |  10 +-
 .../server/persistence/FileTxnLog.java          |  22 +-
 .../server/persistence/FileTxnSnapLog.java      |  46 ++++
 .../zookeeper/server/persistence/Util.java      |  28 ++-
 .../server/persistence/FileTxnSnapLogTest.java  | 244 ++++++++++++++-----
 5 files changed, 265 insertions(+), 85 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/bebe4162/src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java
----------------------------------------------------------------------
diff --git 
a/src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java 
b/src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java
index 2ea714e..5cdfadb 100644
--- a/src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java
+++ b/src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java
@@ -58,6 +58,8 @@ public class FileSnap implements SnapShot {
     public final static int SNAP_MAGIC
             = ByteBuffer.wrap("ZKSN".getBytes()).getInt();
 
+    public static final String SNAPSHOT_FILE_PREFIX = "snapshot";
+
     public FileSnap(File snapDir) {
         this.snapDir = snapDir;
     }
@@ -98,7 +100,7 @@ public class FileSnap implements SnapShot {
         if (!foundValid) {
             throw new IOException("Not able to find valid snapshots in " + 
snapDir);
         }
-        dt.lastProcessedZxid = Util.getZxidFromName(snap.getName(), 
"snapshot");
+        dt.lastProcessedZxid = Util.getZxidFromName(snap.getName(), 
SNAPSHOT_FILE_PREFIX);
         return dt.lastProcessedZxid;
     }
 
@@ -146,7 +148,7 @@ public class FileSnap implements SnapShot {
      * @throws IOException
      */
     private List<File> findNValidSnapshots(int n) throws IOException {
-        List<File> files = Util.sortDataDir(snapDir.listFiles(),"snapshot", 
false);
+        List<File> files = Util.sortDataDir(snapDir.listFiles(), 
SNAPSHOT_FILE_PREFIX, false);
         int count = 0;
         List<File> list = new ArrayList<File>();
         for (File f : files) {
@@ -176,13 +178,13 @@ public class FileSnap implements SnapShot {
      * @throws IOException
      */
     public List<File> findNRecentSnapshots(int n) throws IOException {
-        List<File> files = Util.sortDataDir(snapDir.listFiles(), "snapshot", 
false);
+        List<File> files = Util.sortDataDir(snapDir.listFiles(), 
SNAPSHOT_FILE_PREFIX, false);
         int count = 0;
         List<File> list = new ArrayList<File>();
         for (File f: files) {
             if (count == n)
                 break;
-            if (Util.getZxidFromName(f.getName(), "snapshot") != -1) {
+            if (Util.getZxidFromName(f.getName(), SNAPSHOT_FILE_PREFIX) != -1) 
{
                 count++;
                 list.add(f);
             }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/bebe4162/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
----------------------------------------------------------------------
diff --git 
a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java 
b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
index 9edc38e..72ec606 100644
--- a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
+++ b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
@@ -99,6 +99,8 @@ public class FileTxnLog implements TxnLog {
 
     public final static int VERSION = 2;
 
+    public static final String LOG_FILE_PREFIX = "log";
+
     /** Maximum time we allow for elapsed fsync before WARNing */
     private final static long fsyncWarningThresholdMS;
 
@@ -208,12 +210,10 @@ public class FileTxnLog implements TxnLog {
         }
         if (logStream==null) {
            if(LOG.isInfoEnabled()){
-                LOG.info("Creating new log file: log." +
-                        Long.toHexString(hdr.getZxid()));
+                LOG.info("Creating new log file: " + 
Util.makeLogName(hdr.getZxid()));
            }
 
-           logFileWrite = new File(logDir, ("log." +
-                   Long.toHexString(hdr.getZxid())));
+           logFileWrite = new File(logDir, Util.makeLogName(hdr.getZxid()));
            fos = new FileOutputStream(logFileWrite);
            logStream=new BufferedOutputStream(fos);
            oa = BinaryOutputArchive.getArchive(logStream);
@@ -290,12 +290,12 @@ public class FileTxnLog implements TxnLog {
      * @return
      */
     public static File[] getLogFiles(File[] logDirList,long snapshotZxid) {
-        List<File> files = Util.sortDataDir(logDirList, "log", true);
+        List<File> files = Util.sortDataDir(logDirList, LOG_FILE_PREFIX, true);
         long logZxid = 0;
         // Find the log file that starts before or at the same time as the
         // zxid of the snapshot
         for (File f : files) {
-            long fzxid = Util.getZxidFromName(f.getName(), "log");
+            long fzxid = Util.getZxidFromName(f.getName(), LOG_FILE_PREFIX);
             if (fzxid > snapshotZxid) {
                 continue;
             }
@@ -307,7 +307,7 @@ public class FileTxnLog implements TxnLog {
         }
         List<File> v=new ArrayList<File>(5);
         for (File f : files) {
-            long fzxid = Util.getZxidFromName(f.getName(), "log");
+            long fzxid = Util.getZxidFromName(f.getName(), LOG_FILE_PREFIX);
             if (fzxid < logZxid) {
                 continue;
             }
@@ -324,7 +324,7 @@ public class FileTxnLog implements TxnLog {
     public long getLastLoggedZxid() {
         File[] files = getLogFiles(logDir.listFiles(), 0);
         long maxLog=files.length>0?
-                Util.getZxidFromName(files[files.length-1].getName(),"log"):-1;
+                
Util.getZxidFromName(files[files.length-1].getName(),LOG_FILE_PREFIX):-1;
 
         // if a log file is more recent we must scan it to find
         // the highest zxid
@@ -622,13 +622,13 @@ public class FileTxnLog implements TxnLog {
          */
         void init() throws IOException {
             storedFiles = new ArrayList<File>();
-            List<File> files = 
Util.sortDataDir(FileTxnLog.getLogFiles(logDir.listFiles(), 0), "log", false);
+            List<File> files = 
Util.sortDataDir(FileTxnLog.getLogFiles(logDir.listFiles(), 0), 
LOG_FILE_PREFIX, false);
             for (File f: files) {
-                if (Util.getZxidFromName(f.getName(), "log") >= zxid) {
+                if (Util.getZxidFromName(f.getName(), LOG_FILE_PREFIX) >= 
zxid) {
                     storedFiles.add(f);
                 }
                 // add the last logfile that is less than the zxid
-                else if (Util.getZxidFromName(f.getName(), "log") < zxid) {
+                else if (Util.getZxidFromName(f.getName(), LOG_FILE_PREFIX) < 
zxid) {
                     storedFiles.add(f);
                     break;
                 }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/bebe4162/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
----------------------------------------------------------------------
diff --git 
a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java 
b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
index 3a03c81..3ca1781 100644
--- a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
+++ b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
@@ -19,6 +19,7 @@
 package org.apache.zookeeper.server.persistence;
 
 import java.io.File;
+import java.io.FilenameFilter;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.util.List;
@@ -136,6 +137,13 @@ public class FileTxnSnapLog {
             throw new DatadirException("Cannot write to snap directory " + 
this.snapDir);
         }
 
+        // check content of transaction log and snapshot dirs if they are two 
different directories
+        // See ZOOKEEPER-2967 for more details
+        if(!this.dataDir.getPath().equals(this.snapDir.getPath())){
+            checkLogDir();
+            checkSnapDir();
+        }
+
         txnLog = new FileTxnLog(this.dataDir);
         snapLog = new FileSnap(this.snapDir);
 
@@ -143,6 +151,30 @@ public class FileTxnSnapLog {
                 ZOOKEEPER_DB_AUTOCREATE_DEFAULT));
     }
 
+    private void checkLogDir() throws LogDirContentCheckException {
+        File[] files = this.dataDir.listFiles(new FilenameFilter() {
+            @Override
+            public boolean accept(File dir, String name) {
+                return Util.isSnapshotFileName(name);
+            }
+        });
+        if (files != null && files.length > 0) {
+            throw new LogDirContentCheckException("Log directory has snapshot 
files. Check if dataLogDir and dataDir configuration is correct.");
+        }
+    }
+
+    private void checkSnapDir() throws SnapDirContentCheckException {
+        File[] files = this.snapDir.listFiles(new FilenameFilter() {
+            @Override
+            public boolean accept(File dir, String name) {
+                return Util.isLogFileName(name);
+            }
+        });
+        if (files != null && files.length > 0) {
+            throw new SnapDirContentCheckException("Snapshot directory has log 
files. Check if dataLogDir and dataDir configuration is correct.");
+        }
+    }
+
     /**
      * get the datadir used by this filetxn
      * snap log
@@ -465,4 +497,18 @@ public class FileTxnSnapLog {
             super(msg, e);
         }
     }
+
+    @SuppressWarnings("serial")
+    public static class LogDirContentCheckException extends DatadirException {
+        public LogDirContentCheckException(String msg) {
+            super(msg);
+        }
+    }
+
+    @SuppressWarnings("serial")
+    public static class SnapDirContentCheckException extends DatadirException {
+        public SnapDirContentCheckException(String msg) {
+            super(msg);
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/bebe4162/src/java/main/org/apache/zookeeper/server/persistence/Util.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/persistence/Util.java 
b/src/java/main/org/apache/zookeeper/server/persistence/Util.java
index bf1abaa..d8dedb9 100644
--- a/src/java/main/org/apache/zookeeper/server/persistence/Util.java
+++ b/src/java/main/org/apache/zookeeper/server/persistence/Util.java
@@ -50,7 +50,7 @@ public class Util {
     private static final String SNAP_DIR="snapDir";
     private static final String LOG_DIR="logDir";
     private static final String DB_FORMAT_CONV="dbFormatConversion";
-    
+
     public static String makeURIString(String dataDir, String dataLogDir, 
             String convPolicy){
         String uri="file:"+SNAP_DIR+"="+dataDir+";"+LOG_DIR+"="+dataLogDir;
@@ -83,7 +83,7 @@ public class Util {
      * @return file name
      */
     public static String makeLogName(long zxid) {
-        return "log." + Long.toHexString(zxid);
+        return FileTxnLog.LOG_FILE_PREFIX + "." + Long.toHexString(zxid);
     }
 
     /**
@@ -93,7 +93,7 @@ public class Util {
      * @return file name
      */
     public static String makeSnapshotName(long zxid) {
-        return "snapshot." + Long.toHexString(zxid);
+        return FileSnap.SNAPSHOT_FILE_PREFIX + "." + Long.toHexString(zxid);
     }
     
     /**
@@ -157,7 +157,7 @@ public class Util {
      * @throws IOException
      */
     public static boolean isValidSnapshot(File f) throws IOException {
-        if (f==null || Util.getZxidFromName(f.getName(), "snapshot") == -1)
+        if (f==null || Util.getZxidFromName(f.getName(), 
FileSnap.SNAPSHOT_FILE_PREFIX) == -1)
             return false;
 
         // Check for a valid snapshot
@@ -294,5 +294,25 @@ public class Util {
         Collections.sort(filelist, new DataDirFileComparator(prefix, 
ascending));
         return filelist;
     }
+
+    /**
+     * Returns true if fileName is a log file name.
+     *
+     * @param fileName
+     * @return
+     */
+    public static boolean isLogFileName(String fileName) {
+        return fileName.startsWith(FileTxnLog.LOG_FILE_PREFIX + ".");
+    }
+
+    /**
+     * Returns true if fileName is a snapshot file name.
+     *
+     * @param fileName
+     * @return
+     */
+    public static boolean isSnapshotFileName(String fileName) {
+        return fileName.startsWith(FileSnap.SNAPSHOT_FILE_PREFIX + ".");
+    }
     
 }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/bebe4162/src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java
----------------------------------------------------------------------
diff --git 
a/src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
b/src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java
index 7a80efd..2159558 100644
--- 
a/src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java
+++ 
b/src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java
@@ -23,8 +23,11 @@ import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.server.DataTree;
 import org.apache.zookeeper.server.Request;
 import org.apache.zookeeper.test.ClientBase;
+import org.apache.zookeeper.test.TestUtils;
 import org.apache.zookeeper.txn.SetDataTxn;
 import org.apache.zookeeper.txn.TxnHeader;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -35,23 +38,84 @@ import java.util.concurrent.ConcurrentHashMap;
 
 public class FileTxnSnapLogTest {
 
-    /**
-     * Test verifies the auto creation of data dir and data log dir.
-     * Sets "zookeeper.datadir.autocreate" to true.
-     */
-    @Test
-    public void testWithAutoCreateDataLogDir() throws IOException {
-        File tmpDir = ClientBase.createEmptyTestDir();
-        File dataDir = new File(tmpDir, "data");
-        File snapDir = new File(tmpDir, "data_txnlog");
-        Assert.assertFalse("data directory already exists", dataDir.exists());
-        Assert.assertFalse("snapshot directory already exists", 
snapDir.exists());
+    private File tmpDir;
+
+    private File logDir;
+
+    private File snapDir;
+
+    private File logVersionDir;
+
+    private File snapVersionDir;
+
+    @Before
+    public void setUp() throws Exception {
+        tmpDir = ClientBase.createEmptyTestDir();
+        logDir = new File(tmpDir, "logdir");
+        snapDir = new File(tmpDir, "snapdir");
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        if(tmpDir != null){
+            TestUtils.deleteFileRecursively(tmpDir);
+        }
+        this.tmpDir = null;
+        this.logDir = null;
+        this.snapDir = null;
+        this.logVersionDir = null;
+        this.snapVersionDir = null;
+    }
+
+    private File createVersionDir(File parentDir) {
+        File versionDir = new File(parentDir, FileTxnSnapLog.version + 
FileTxnSnapLog.VERSION);
+        versionDir.mkdirs();
+        return versionDir;
+    }
+
+    private void createLogFile(File dir, long zxid) throws IOException {
+        File file = new File(dir.getPath() + File.separator + 
Util.makeLogName(zxid));
+        file.createNewFile();
+    }
+
+    private void createSnapshotFile(File dir, long zxid) throws IOException {
+        File file = new File(dir.getPath() + File.separator + 
Util.makeSnapshotName(zxid));
+        file.createNewFile();
+    }
+
+    private void twoDirSetupWithCorrectFiles() throws IOException {
+        logVersionDir = createVersionDir(logDir);
+        snapVersionDir = createVersionDir(snapDir);
 
+        // transaction log files in log dir
+        createLogFile(logVersionDir,1);
+        createLogFile(logVersionDir,2);
+
+        // snapshot files in snap dir
+        createSnapshotFile(snapVersionDir,1);
+        createSnapshotFile(snapVersionDir,2);
+    }
+
+    private void singleDirSetupWithCorrectFiles() throws IOException {
+        logVersionDir = createVersionDir(logDir);
+
+        // transaction log and snapshot files in the same dir
+        createLogFile(logVersionDir,1);
+        createLogFile(logVersionDir,2);
+        createSnapshotFile(logVersionDir,1);
+        createSnapshotFile(logVersionDir,2);
+    }
+
+    private FileTxnSnapLog createFileTxnSnapLogWithNoAutoCreateDataDir(File 
logDir, File snapDir) throws IOException {
+        return createFileTxnSnapLogWithAutoCreateDataDir(logDir, snapDir, 
"false");
+    }
+
+    private FileTxnSnapLog createFileTxnSnapLogWithAutoCreateDataDir(File 
logDir, File snapDir, String autoCreateValue) throws IOException {
         String priorAutocreateDirValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
-        System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
"true");
+        System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
autoCreateValue);
         FileTxnSnapLog fileTxnSnapLog;
         try {
-            fileTxnSnapLog = new FileTxnSnapLog(dataDir, snapDir);
+            fileTxnSnapLog = new FileTxnSnapLog(logDir, snapDir);
         } finally {
             if (priorAutocreateDirValue == null) {
                 
System.clearProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
@@ -59,68 +123,96 @@ public class FileTxnSnapLogTest {
                 
System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
priorAutocreateDirValue);
             }
         }
-        Assert.assertTrue(dataDir.exists());
+        return fileTxnSnapLog;
+    }
+
+    private FileTxnSnapLog createFileTxnSnapLogWithAutoCreateDB(File logDir, 
File snapDir, String autoCreateValue) throws IOException {
+        String priorAutocreateDBValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE);
+        System.setProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE, 
autoCreateValue);
+        FileTxnSnapLog fileTxnSnapLog;
+        try {
+            fileTxnSnapLog = new FileTxnSnapLog(logDir, snapDir);
+        } finally {
+            if (priorAutocreateDBValue == null) {
+                System.clearProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE);
+            } else {
+                System.setProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE, 
priorAutocreateDBValue);
+            }
+        }
+        return fileTxnSnapLog;
+    }
+
+    /**
+     * Test verifies the auto creation of log dir and snap dir.
+     * Sets "zookeeper.datadir.autocreate" to true.
+     */
+    @Test
+    public void testWithAutoCreateDataDir() throws IOException {
+        Assert.assertFalse("log directory already exists", logDir.exists());
+        Assert.assertFalse("snapshot directory already exists", 
snapDir.exists());
+
+        FileTxnSnapLog fileTxnSnapLog = 
createFileTxnSnapLogWithAutoCreateDataDir(logDir, snapDir, "true");
+
+        Assert.assertTrue(logDir.exists());
         Assert.assertTrue(snapDir.exists());
         Assert.assertTrue(fileTxnSnapLog.getDataDir().exists());
         Assert.assertTrue(fileTxnSnapLog.getSnapDir().exists());
     }
 
     /**
-     * Test verifies server should fail when data dir or data log dir doesn't
-     * exists. Sets "zookeeper.datadir.autocreate" to false.
+     * Test verifies server should fail when log dir or snap dir doesn't exist.
+     * Sets "zookeeper.datadir.autocreate" to false.
      */
-    @Test
-    public void testWithoutAutoCreateDataLogDir() throws Exception {
-        File tmpDir = ClientBase.createEmptyTestDir();
-        File dataDir = new File(tmpDir, "data");
-        File snapDir = new File(tmpDir, "data_txnlog");
-        Assert.assertFalse("data directory already exists", dataDir.exists());
+    @Test(expected = FileTxnSnapLog.DatadirException.class)
+    public void testWithoutAutoCreateDataDir() throws Exception {
+        Assert.assertFalse("log directory already exists", logDir.exists());
         Assert.assertFalse("snapshot directory already exists", 
snapDir.exists());
 
-        String priorAutocreateDirValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
-        System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
"false");
         try {
-            FileTxnSnapLog fileTxnSnapLog = new FileTxnSnapLog(dataDir, 
snapDir);
+            createFileTxnSnapLogWithAutoCreateDataDir(logDir, snapDir, 
"false");
         } catch (FileTxnSnapLog.DatadirException e) {
-            Assert.assertFalse(dataDir.exists());
+            Assert.assertFalse(logDir.exists());
             Assert.assertFalse(snapDir.exists());
-            return;
-        } finally {
-            if (priorAutocreateDirValue == null) {
-                
System.clearProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
-            } else {
-                
System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
priorAutocreateDirValue);
-            }
+            // rethrow exception
+            throw e;
         }
         Assert.fail("Expected exception from FileTxnSnapLog");
     }
 
+    private void attemptAutoCreateDB(File dataDir, File snapDir, Map<Long, 
Integer> sessions,
+                                     String autoCreateValue, long 
expectedValue) throws IOException {
+        sessions.clear();
+
+        FileTxnSnapLog fileTxnSnapLog = 
createFileTxnSnapLogWithAutoCreateDB(dataDir, snapDir, autoCreateValue);
+
+        long zxid = fileTxnSnapLog.restore(new DataTree(), sessions, new 
FileTxnSnapLog.PlayBackListener() {
+            @Override
+            public void onTxnLoaded(TxnHeader hdr, Record rec) {
+                // empty by default
+            }
+        });
+        Assert.assertEquals("unexpected zxid", expectedValue, zxid);
+    }
+
     @Test
-    public void testAutoCreateDb() throws IOException {
-        File tmpDir = ClientBase.createEmptyTestDir();
-        File dataDir = new File(tmpDir, "data");
-        File snapDir = new File(tmpDir, "data_txnlog");
-        Assert.assertTrue("cannot create data directory", dataDir.mkdir());
+    public void testAutoCreateDB() throws IOException {
+        Assert.assertTrue("cannot create log directory", logDir.mkdir());
         Assert.assertTrue("cannot create snapshot directory", snapDir.mkdir());
-        File initFile = new File(dataDir, "initialize");
+        File initFile = new File(logDir, "initialize");
         Assert.assertFalse("initialize file already exists", 
initFile.exists());
 
-        String priorAutocreateDbValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE);
         Map<Long, Integer> sessions = new ConcurrentHashMap<>();
 
-        attemptAutoCreateDb(dataDir, snapDir, sessions, 
priorAutocreateDbValue, "false", -1L);
-
-        attemptAutoCreateDb(dataDir, snapDir, sessions, 
priorAutocreateDbValue, "true", 0L);
+        attemptAutoCreateDB(logDir, snapDir, sessions,"false", -1L);
+        attemptAutoCreateDB(logDir, snapDir, sessions,"true", 0L);
 
         Assert.assertTrue("cannot create initialize file", 
initFile.createNewFile());
-        attemptAutoCreateDb(dataDir, snapDir, sessions, 
priorAutocreateDbValue, "false", 0L);
+        attemptAutoCreateDB(logDir, snapDir, sessions,"false", 0L);
     }
 
     @Test
     public void testGetTxnLogSyncElapsedTime() throws IOException {
-        File tmpDir = ClientBase.createEmptyTestDir();
-        FileTxnSnapLog fileTxnSnapLog = new FileTxnSnapLog(new File(tmpDir, 
"data"),
-                new File(tmpDir, "data_txnlog"));
+        FileTxnSnapLog fileTxnSnapLog = 
createFileTxnSnapLogWithAutoCreateDataDir(logDir, snapDir, "true");
 
         TxnHeader hdr = new TxnHeader(1, 1, 1, 1, ZooDefs.OpCode.setData);
         Record txn = new SetDataTxn("/foo", new byte[0], 1);
@@ -136,27 +228,47 @@ public class FileTxnSnapLogTest {
         }
     }
 
-    private void attemptAutoCreateDb(File dataDir, File snapDir, Map<Long, 
Integer> sessions,
-                                     String priorAutocreateDbValue, String 
autoCreateValue,
-                                     long expectedValue) throws IOException {
-        sessions.clear();
-        System.setProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE, 
autoCreateValue);
-        FileTxnSnapLog fileTxnSnapLog = new FileTxnSnapLog(dataDir, snapDir);
+    @Test
+    public void testDirCheckWithCorrectFiles() throws IOException {
+        twoDirSetupWithCorrectFiles();
 
         try {
-            long zxid = fileTxnSnapLog.restore(new DataTree(), sessions, new 
FileTxnSnapLog.PlayBackListener() {
-                @Override
-                public void onTxnLoaded(TxnHeader hdr, Record rec) {
-                    // empty by default
-                }
-            });
-            Assert.assertEquals("unexpected zxid", expectedValue, zxid);
-        } finally {
-            if (priorAutocreateDbValue == null) {
-                System.clearProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE);
-            } else {
-                System.setProperty(FileTxnSnapLog.ZOOKEEPER_DB_AUTOCREATE, 
priorAutocreateDbValue);
-            }
+            createFileTxnSnapLogWithNoAutoCreateDataDir(logDir, snapDir);
+        } catch (FileTxnSnapLog.LogDirContentCheckException | 
FileTxnSnapLog.SnapDirContentCheckException e) {
+            Assert.fail("Should not throw ContentCheckException.");
         }
     }
+
+    @Test
+    public void testDirCheckWithSingleDirSetup() throws IOException {
+        singleDirSetupWithCorrectFiles();
+
+        try {
+            createFileTxnSnapLogWithNoAutoCreateDataDir(logDir, logDir);
+        } catch (FileTxnSnapLog.LogDirContentCheckException | 
FileTxnSnapLog.SnapDirContentCheckException e) {
+            Assert.fail("Should not throw ContentCheckException.");
+        }
+    }
+
+    @Test(expected = FileTxnSnapLog.LogDirContentCheckException.class)
+    public void testDirCheckWithSnapFilesInLogDir() throws IOException {
+        twoDirSetupWithCorrectFiles();
+
+        // add snapshot files to the log version dir
+        createSnapshotFile(logVersionDir,3);
+        createSnapshotFile(logVersionDir,4);
+
+        createFileTxnSnapLogWithNoAutoCreateDataDir(logDir, snapDir);
+    }
+
+    @Test(expected = FileTxnSnapLog.SnapDirContentCheckException.class)
+    public void testDirCheckWithLogFilesInSnapDir() throws IOException {
+        twoDirSetupWithCorrectFiles();
+
+        // add transaction log files to the snap version dir
+        createLogFile(snapVersionDir,3);
+        createLogFile(snapVersionDir,4);
+
+        createFileTxnSnapLogWithNoAutoCreateDataDir(logDir, snapDir);
+    }
 }

Reply via email to