You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by mfenes <gi...@git.apache.org> on 2018/02/07 15:30:44 UTC
[GitHub] zookeeper pull request #458: ZOOKEEPER-2967: Add check to validate dataDir a...
GitHub user mfenes opened a pull request:
https://github.com/apache/zookeeper/pull/458
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:
transaction log and snapshot directories are different and they are used correctly (no Exception)
transaction log and snapshot directories are the same (in this case no check is done)
transaction log and snapshot directories are different and transaction log directory contains snapshot files (LogdirContentCheckException -> ZK quits)
transaction log and snapshot directories are different and snapshot directory contains transaction log files (SnapdirContentCheckException -> ZK quits)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mfenes/zookeeper ZOOKEEPER-2967_3.5
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/458.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #458
----
commit 9d7c91e298ca7a291f502fdc792a2a27bd066d31
Author: Mark Fenes <mf...@...>
Date: 2018-01-19T23:16:07Z
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at startup
commit 1fc1eb4fadf3ce2d83e3cd81d97332e9bf96f95c
Author: Mark Fenes <mf...@...>
Date: 2018-01-20T00:57:13Z
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at startup
Replaced |= as this operator is not short-circuit
commit 9ea2a73fe0719070bbf9411752dc4b08df5f01c4
Author: Mark Fenes <mf...@...>
Date: 2018-01-29T23:08:11Z
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at startup
FilenameFilter, includes dot in log/snapshot filename check, improved/shortened unit test code,
added @Before and @After to set up and delete temporary test directories.
commit 19f1b67449825037cc266f1ea5999a82d7baa352
Author: Mark Fenes <mf...@...>
Date: 2018-02-01T23:21:45Z
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at startup
Replaced "log" and "snapshot" strings with constants FileTxnLog.LOG_FILE_PREFIX and FileSnap.SNAPSHOT_FILE_PREFIX.
Used Util.makeLogName when creating new transaction log file to make sure creation and verification follows the same file name pattern. Added reference to ZOOKEEPER-2967 to explain why this check is needed.
Cleanup/refactoring of unit test FileTxnSnapLogTest.
commit 9b04f3b1e5b05eed80ba2c2df90770fb35bafbef
Author: Mark Fenes <mf...@...>
Date: 2018-02-07T12:53:35Z
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at startup
Fixed possible NP issues.
commit 4468470a9483468a739a2cc7583e9d10ef7f233b
Author: Mark Fenes <mf...@...>
Date: 2018-02-07T15:24:42Z
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters at startup
Required changes to backport ZOOKEEPER-2967 from master to 3.5.
----
---
[GitHub] zookeeper pull request #458: ZOOKEEPER-2967: Add check to validate dataDir a...
Posted by mfenes <gi...@git.apache.org>.
Github user mfenes commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/458#discussion_r168888396
--- Diff: src/java/test/org/apache/zookeeper/test/ClientBase.java ---
@@ -368,22 +368,37 @@ static void verifyThreadTerminated(Thread thread, long millis)
}
}
+ public static File createEmptyTestDir() throws IOException {
+ return createTmpDir(BASETEST, false);
+ }
public static File createTmpDir() throws IOException {
- return createTmpDir(BASETEST);
+ return createTmpDir(BASETEST, true);
}
- static File createTmpDir(File parentDir) throws IOException {
+ static File createTmpDir(File parentDir, boolean createInitFile) throws IOException {
File tmpFile = File.createTempFile("test", ".junit", parentDir);
// don't delete tmpFile - this ensures we don't attempt to create
// a tmpDir with a duplicate name
File tmpDir = new File(tmpFile + ".dir");
Assert.assertFalse(tmpDir.exists()); // never true if tmpfile does it's job
Assert.assertTrue(tmpDir.mkdirs());
+ // todo not every tmp directory needs this file
--- End diff --
Unfortunately this todo comment is in the master branch from where I backported these changes in ClientBase.
---
[GitHub] zookeeper issue #458: ZOOKEEPER-2967: Add check to validate dataDir and data...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on the issue:
https://github.com/apache/zookeeper/pull/458
@mfenes Thank you, please close this PR
---
[GitHub] zookeeper pull request #458: ZOOKEEPER-2967: Add check to validate dataDir a...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/458#discussion_r168301155
--- Diff: src/java/test/org/apache/zookeeper/test/ClientBase.java ---
@@ -368,22 +368,37 @@ static void verifyThreadTerminated(Thread thread, long millis)
}
}
+ public static File createEmptyTestDir() throws IOException {
+ return createTmpDir(BASETEST, false);
+ }
public static File createTmpDir() throws IOException {
- return createTmpDir(BASETEST);
+ return createTmpDir(BASETEST, true);
}
- static File createTmpDir(File parentDir) throws IOException {
+ static File createTmpDir(File parentDir, boolean createInitFile) throws IOException {
File tmpFile = File.createTempFile("test", ".junit", parentDir);
// don't delete tmpFile - this ensures we don't attempt to create
// a tmpDir with a duplicate name
File tmpDir = new File(tmpFile + ".dir");
Assert.assertFalse(tmpDir.exists()); // never true if tmpfile does it's job
Assert.assertTrue(tmpDir.mkdirs());
+ // todo not every tmp directory needs this file
--- End diff --
why is this todo here?
---
[GitHub] zookeeper pull request #458: ZOOKEEPER-2967: Add check to validate dataDir a...
Posted by mfenes <gi...@git.apache.org>.
Github user mfenes closed the pull request at:
https://github.com/apache/zookeeper/pull/458
---