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 2ea714e47f3..5cdfadbf0d9 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 long deserialize(DataTree dt, Map sessions) 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 File findMostRecentSnapshot() throws IOException { * @throws IOException */ private List findNValidSnapshots(int n) throws IOException { - List files = Util.sortDataDir(snapDir.listFiles(),"snapshot", false); + List files = Util.sortDataDir(snapDir.listFiles(), SNAPSHOT_FILE_PREFIX, false); int count = 0; List list = new ArrayList(); for (File f : files) { @@ -176,13 +178,13 @@ private List findNValidSnapshots(int n) throws IOException { * @throws IOException */ public List findNRecentSnapshots(int n) throws IOException { - List files = Util.sortDataDir(snapDir.listFiles(), "snapshot", false); + List files = Util.sortDataDir(snapDir.listFiles(), SNAPSHOT_FILE_PREFIX, false); int count = 0; List list = new ArrayList(); 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); } 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 9edc38e5095..72ec606a3ae 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 synchronized boolean append(TxnHeader hdr, Record txn) } 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 static long calculateFileSizeWithPadding(long position, long fileSize, lo * @return */ public static File[] getLogFiles(File[] logDirList,long snapshotZxid) { - List files = Util.sortDataDir(logDirList, "log", true); + List 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 static File[] getLogFiles(File[] logDirList,long snapshotZxid) { } List v=new ArrayList(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 static File[] getLogFiles(File[] logDirList,long snapshotZxid) { 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 FileTxnIterator(File logDir, long zxid) throws IOException { */ void init() throws IOException { storedFiles = new ArrayList(); - List files = Util.sortDataDir(FileTxnLog.getLogFiles(logDir.listFiles(), 0), "log", false); + List 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; } 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 3a03c81e3a7..3ca1781424e 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 FileTxnSnapLog(File dataDir, File snapDir) throws IOException { 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 FileTxnSnapLog(File dataDir, File snapDir) throws IOException { 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 DatadirException(String msg, Exception e) { 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); + } + } } 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 bf1abaafebd..d8dedb98f4d 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 static URI makeFileLoggerURL(File dataDir, File dataLogDir,String convPol * @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 static String makeLogName(long zxid) { * @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 static long getZxidFromName(String name, String prefix) { * @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 static List sortDataDir(File[] files, String prefix, boolean ascend 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 + "."); + } } 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 7a80efd4243..21595581d35 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.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 @@ 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 void testWithAutoCreateDataLogDir() throws IOException { 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 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 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 void testGetTxnLogSyncElapsedTime() throws IOException { } } - private void attemptAutoCreateDb(File dataDir, File snapDir, Map 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); + } }