Skip to content

Commit

Permalink
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir paramete…
Browse files Browse the repository at this point in the history 
…rs 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 <mfenes@cloudera.com>

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

Closes apache#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
  • Loading branch information
@afine
Mark Fenes authored and afine committed Feb 14, 2018
1 parent aefb13f commit bebe416
Show file tree
Hide file tree
Showing 5 changed files with 265 additions and 85 deletions.
10 changes: 6 additions & 4 deletions src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -98,7 +100,7 @@ public long deserialize(DataTree dt, Map<Long, Integer> 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;
}

Expand Down Expand Up @@ -146,7 +148,7 @@ public File findMostRecentSnapshot() throws IOException {
* @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) {
Expand Down Expand Up @@ -176,13 +178,13 @@ private List<File> findNValidSnapshots(int n) throws IOException {
* @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);
}
Expand Down
22 changes: 11 additions & 11 deletions src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -290,12 +290,12 @@ public static long calculateFileSizeWithPadding(long position, long fileSize, lo
* @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;
}
Expand All @@ -307,7 +307,7 @@ public static File[] getLogFiles(File[] logDirList,long snapshotZxid) {
}
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;
}
Expand All @@ -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
Expand Down Expand Up @@ -622,13 +622,13 @@ public FileTxnIterator(File logDir, long zxid) throws IOException {
*/
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;
}
Expand Down
46 changes: 46 additions & 0 deletions src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -136,13 +137,44 @@ 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);

autoCreateDB = Boolean.parseBoolean(System.getProperty(ZOOKEEPER_DB_AUTOCREATE,
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
Expand Down Expand Up @@ -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);
}
}
}
28 changes: 24 additions & 4 deletions src/java/main/org/apache/zookeeper/server/persistence/Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -294,5 +294,25 @@ public static List<File> 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 + ".");
}

}
Loading

0 comments on commit bebe416

Please sign in to comment.