subject:"\[jira\] \[Commented\] \(ZOOKEEPER\-2967\) Add check to validate dataDir and dataLogDir parameters at startup"

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-05-11 Thread Mauricio Garavaglia (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16472554#comment-16472554
 ] 

Mauricio Garavaglia commented on ZOOKEEPER-2967:


This broke the upgrade path between 3.9.11 and 3.9.12, requiring manual 
intervention. That needs to be specified in the release notes.

> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16371582#comment-16371582
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes closed the pull request at:

https://github.com/apache/zookeeper/pull/459


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-21 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16371580#comment-16371580
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes closed the pull request at:

https://github.com/apache/zookeeper/pull/458


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-20 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16370907#comment-16370907
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user afine commented on the issue:

https://github.com/apache/zookeeper/pull/459
  
@mfenes Thank you, please close this PR


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-20 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16370905#comment-16370905
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user afine commented on the issue:

https://github.com/apache/zookeeper/pull/458
  
@mfenes Thank you, please close this PR


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-16 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16367982#comment-16367982
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r168891236
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -35,92 +38,181 @@
 
 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();
--- End diff --

@eolivelli Thanks for the idea. I'll check and use this next time.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-16 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16367972#comment-16367972
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/459#discussion_r168889616
  
--- Diff: src/java/test/org/apache/zookeeper/test/ClientBase.java ---
@@ -351,20 +351,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.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-16 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16367959#comment-16367959
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/458#discussion_r16396
  
--- 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. 


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-16 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16367304#comment-16367304
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on the issue:

https://github.com/apache/zookeeper/pull/459
  
@afine I've removed the whitespace changes. Please have a look if it looks 
right now.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-14 Thread Hudson (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364805#comment-16364805
 ] 

Hudson commented on ZOOKEEPER-2967:
---

SUCCESS: Integrated in Jenkins build ZooKeeper-trunk #3727 (See 
[https://builds.apache.org/job/ZooKeeper-trunk/3727/])
ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir parameters (afine: 
rev bebe4162bc915b15b4fd90df9446bdf9f83ef3a8)
* (edit) 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java
* (edit) src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
* (edit) src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java
* (edit) 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
* (edit) src/java/main/org/apache/zookeeper/server/persistence/Util.java


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-14 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364734#comment-16364734
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/459#discussion_r168301408
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileSnap.java ---
@@ -51,19 +51,22 @@
 public class FileSnap implements SnapShot {
 File snapDir;
 private volatile boolean close = false;
-private static final int VERSION=2;
-private static final long dbId=-1;
+private static final int VERSION = 2;
--- End diff --

Please revert this


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-14 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364735#comment-16364735
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/459#discussion_r168301575
  
--- Diff: src/java/test/org/apache/zookeeper/test/ClientBase.java ---
@@ -351,20 +351,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 --

do we need this todo?


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-14 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364732#comment-16364732
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user eolivelli commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r168301786
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -35,92 +38,181 @@
 
 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();
--- End diff --

Maybe it could be a good improvement to start using junit TemporaryFolder 
rule so that the clean up will ve automatically handled by junit


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-14 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364728#comment-16364728
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

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?


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-14 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364715#comment-16364715
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/450


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-08 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16356773#comment-16356773
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

GitHub user mfenes opened a pull request:

https://github.com/apache/zookeeper/pull/459

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.4

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/459.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 #459


commit 3cebb86cf5b2ab7561ee1a25e6eb7cc10bb7a5b6
Author: Mark Fenes 
Date:   2018-01-19T23:16:07Z

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

commit a0ef2ce168e65919a2613e8e7dcb0effaf3f838a
Author: Mark Fenes 
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 3d978556fc525c9f93db5c73a59d2cf52f847d6c
Author: Mark Fenes 
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 4096a2500961e3015a8ec009ecfa088b6642a1e8
Author: Mark Fenes 
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 5d8b5d0462700c70d317bc3de3989a03081cba36
Author: Mark Fenes 
Date:   2018-02-07T12:53:35Z

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

Fixed possible NP issues.

commit 666a1b0a1bd33406f26c29df1407f75c94675d0f
Author: Mark Fenes 
Date:   2018-02-08T10:31:46Z

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

Required changes to backport ZOOKEEPER-2967 from master to 3.4.




> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-07 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16355599#comment-16355599
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

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 
Date:   2018-01-19T23:16:07Z

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

commit 1fc1eb4fadf3ce2d83e3cd81d97332e9bf96f95c
Author: Mark Fenes 
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 
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 
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 
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 
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.




> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-01 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16349544#comment-16349544
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r165525848
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
--- End diff --

Added reference to ZOOKEEPER-2967 in the comments.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-02-01 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16349541#comment-16349541
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on the issue:

https://github.com/apache/zookeeper/pull/450
  
@afine I've made the following changes:
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.
Cleaned up/refactored unit test FileTxnSnapLogTest to make it less verbose 
and consistent.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344253#comment-16344253
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164606812
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
+
+// transaction log files in log dir - correct
+File logFile1 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(1L));
+logFile1.createNewFile();
+File logFile2 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(2L));
+logFile2.createNewFile();
+
+// snapshot files in snap dir - correct
+File snapFile1 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(1L));
+snapFile1.createNewFile();
+File snapFile2 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(2L));
+snapFile2.createNewFile();
+
+Assert.assertTrue(logFile1.exists());
+Assert.assertTrue(logFile2.exists());
+Assert.assertTrue(snapFile1.exists());
+Assert.assertTrue(snapFile2.exists());
+
+String priorAutocreateDirValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
"false");
+FileTxnSnapLog fileTxnSnapLog;
+try {
+fileTxnSnapLog = new FileTxnSnapLog(logDir, snapDir);
+} catch (FileTxnSnapLog.LogdirContentCheckException e) {
+Assert.fail("Should not throw LogdirContentCheckException.");
+} catch (FileTxnSnapLog.SnapdirContentCheckException e) {
+Assert.fail("Should not throw SnapdirContentCheckException.");
+} finally {
+if (priorAutocreateDirValue == null) {
+
System.clearProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+} else {
+
System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
priorAutocreateDirValue);
+}
--- End diff --

I switched off autocreate to make sure the test is reading the test input 
data and not something else by mistake.
Added clean-up of temp directories.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344242#comment-16344242
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605909
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
--- End diff --

Ok, removed.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344243#comment-16344243
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605964
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
+
+// transaction log files in log dir - correct
+File logFile1 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(1L));
+logFile1.createNewFile();
+File logFile2 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(2L));
+logFile2.createNewFile();
+
+// snapshot files in snap dir - correct
+File snapFile1 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(1L));
+snapFile1.createNewFile();
+File snapFile2 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(2L));
+snapFile2.createNewFile();
+
+Assert.assertTrue(logFile1.exists());
+Assert.assertTrue(logFile2.exists());
+Assert.assertTrue(snapFile1.exists());
+Assert.assertTrue(snapFile2.exists());
--- End diff --

Ok, removed.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344245#comment-16344245
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605998
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
+
+// transaction log files in log dir - correct
+File logFile1 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(1L));
+logFile1.createNewFile();
+File logFile2 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(2L));
+logFile2.createNewFile();
+
+// snapshot files in snap dir - correct
+File snapFile1 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(1L));
+snapFile1.createNewFile();
+File snapFile2 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(2L));
+snapFile2.createNewFile();
+
+Assert.assertTrue(logFile1.exists());
+Assert.assertTrue(logFile2.exists());
+Assert.assertTrue(snapFile1.exists());
+Assert.assertTrue(snapFile2.exists());
+
+String priorAutocreateDirValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
"false");
+FileTxnSnapLog fileTxnSnapLog;
+try {
+fileTxnSnapLog = new FileTxnSnapLog(logDir, snapDir);
+} catch (FileTxnSnapLog.LogdirContentCheckException e) {
+Assert.fail("Should not throw LogdirContentCheckException.");
+} catch (FileTxnSnapLog.SnapdirContentCheckException e) {
--- End diff --

Done.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344241#comment-16344241
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605860
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
--- End diff --

I've removed this, too.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344240#comment-16344240
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605834
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
--- End diff --

I've removed this.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344237#comment-16344237
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605619
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -294,5 +297,25 @@ public int compare(File o1, File o2) {
 Collections.sort(filelist, new DataDirFileComparator(prefix, 
ascending));
 return filelist;
 }
+
+/**
+ * Returns true if file is a log file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isLogFile(File file) {
+return file.getName().startsWith(LOG_FILE_PREFIX);
--- End diff --

This is done.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344238#comment-16344238
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605723
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -294,5 +297,25 @@ public int compare(File o1, File o2) {
 Collections.sort(filelist, new DataDirFileComparator(prefix, 
ascending));
 return filelist;
 }
+
+/**
+ * Returns true if file is a log file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isLogFile(File file) {
+return file.getName().startsWith(LOG_FILE_PREFIX);
+}
+
+/**
+ * Returns true if file is a snapshot file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isSnapshotFile(File file) {
+return file.getName().startsWith(SNAP_FILE_PREFIX);
--- End diff --

Done.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344235#comment-16344235
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605521
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
+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();
--- End diff --

Changed implementation to use FilenameFilter.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-29 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16344234#comment-16344234
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r164605358
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
--- End diff --

@afine I've improved/shortened the test code


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16337084#comment-16337084
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163395510
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -50,6 +50,9 @@
 private static final String SNAP_DIR="snapDir";
 private static final String LOG_DIR="logDir";
 private static final String DB_FORMAT_CONV="dbFormatConversion";
+
+private static final String LOG_FILE_PREFIX = "log";
--- End diff --

Can we use these field when actually creating the log and snapshot files?


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16337085#comment-16337085
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163395848
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
--- End diff --

Could we add a reference to the discussion that explains why this whole 
check is needed in the comments and/or the exception that the user sees?


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16337083#comment-16337083
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163472195
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
--- End diff --

there is a lot of code duplication in these tests, i'm wondering if they 
can be cleaned up.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16336954#comment-16336954
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163457880
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -83,7 +86,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 LOG_FILE_PREFIX + "." + Long.toHexString(zxid);
--- End diff --

Okay.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16336953#comment-16336953
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163457798
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
+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();
--- End diff --

I prefer writing (and later reading) less code than more. You could be 
right about the performance impact, but at least `listFiles()` will only create 
File objects for the matching files, not for all of them.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335970#comment-16335970
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163290319
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -294,5 +297,25 @@ public int compare(File o1, File o2) {
 Collections.sort(filelist, new DataDirFileComparator(prefix, 
ascending));
 return filelist;
 }
+
+/**
+ * Returns true if file is a log file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isLogFile(File file) {
+return file.getName().startsWith(LOG_FILE_PREFIX);
+}
+
+/**
+ * Returns true if file is a snapshot file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isSnapshotFile(File file) {
+return file.getName().startsWith(SNAP_FILE_PREFIX);
--- End diff --

Ok, I'll include the dot in the check.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335969#comment-16335969
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163290119
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -294,5 +297,25 @@ public int compare(File o1, File o2) {
 Collections.sort(filelist, new DataDirFileComparator(prefix, 
ascending));
 return filelist;
 }
+
+/**
+ * Returns true if file is a log file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isLogFile(File file) {
+return file.getName().startsWith(LOG_FILE_PREFIX);
--- End diff --

I agree that it might be safer to check with the dot. I'll add this change.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335966#comment-16335966
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163289284
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -83,7 +86,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 LOG_FILE_PREFIX + "." + Long.toHexString(zxid);
--- End diff --

I deliberately excluded the dot from LOG_FILE_PREFIX and SNAP_FILE_PREFIX 
after I checked that in FileTxnLog and FileSnap classes the prefix argument 
(like "log", "snapshot") in method calls is passed without the dot. I wanted to 
avoid confusion.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335942#comment-16335942
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163282674
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
+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();
+if(files != null) {
+boolean hasSnapshotFiles = false;
+for (File file : files) {
+if(Util.isSnapshotFile(file)){
+hasSnapshotFiles = true;
+break;
+}
+}
+if (hasSnapshotFiles) {
+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();
--- End diff --

Please see my comments above.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335938#comment-16335938
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user mfenes commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r163282090
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
+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();
--- End diff --

If I used FilenameFilter, then Util.isSnapshotFile() / Util.isLogFile() 
check would be run for all the files in the directory and 
listFiles(FilenameFilter filter) would return all the files satisfying the 
filter condition, however I need only the first occurrence which satisfies the 
condition, not all of them. The current logic quits from the for loop 
immediately when it finds a file violating the configuration and throws an 
exception, while your proposal would iterate over all the files in the 
directory and would call Util.isSnapshotFile() / Util.isLogFile() for each of 
the files inside FilenameFilter to prepare the filtered File[]. So using 
FilenameFilter would be a bit slower, but yes, it might need less lines in 
code, also at the price of obscuring the purpose of the code (i.e. 
hasSnapshotFiles / hasLogFiles boolean variables tell what the problem exactly 
is, while if (snapshotFiles.length > 0) { throw new Exception(...) } would 
not). However, if we prefer using Java library classes over standard coding 
patterns even in cases when it does not fit the purpose entirely, then 
FilenameFilter can be the winner.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334334#comment-16334334
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162948359
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
+
+// transaction log files in log dir - correct
+File logFile1 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(1L));
+logFile1.createNewFile();
+File logFile2 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(2L));
+logFile2.createNewFile();
+
+// snapshot files in snap dir - correct
+File snapFile1 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(1L));
+snapFile1.createNewFile();
+File snapFile2 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(2L));
+snapFile2.createNewFile();
+
+Assert.assertTrue(logFile1.exists());
+Assert.assertTrue(logFile2.exists());
+Assert.assertTrue(snapFile1.exists());
+Assert.assertTrue(snapFile2.exists());
+
+String priorAutocreateDirValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
"false");
+FileTxnSnapLog fileTxnSnapLog;
+try {
+fileTxnSnapLog = new FileTxnSnapLog(logDir, snapDir);
+} catch (FileTxnSnapLog.LogdirContentCheckException e) {
+Assert.fail("Should not throw LogdirContentCheckException.");
+} catch (FileTxnSnapLog.SnapdirContentCheckException e) {
+Assert.fail("Should not throw SnapdirContentCheckException.");
+} finally {
+if (priorAutocreateDirValue == null) {
+
System.clearProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+} else {
+
System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
priorAutocreateDirValue);
+}
--- End diff --

Setting 'zookeeper.datadir.autocreate' doesn't make any difference here, so 
it doesn't need to be maintained.
In addition to that please add clean-up logic to your tests. (e.g. 
recursively remove temp directories with all their contents)


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334337#comment-16334337
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162954011
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
+
+// transaction log files in log dir - correct
+File logFile1 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(1L));
+logFile1.createNewFile();
+File logFile2 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(2L));
+logFile2.createNewFile();
+
+// snapshot files in snap dir - correct
+File snapFile1 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(1L));
+snapFile1.createNewFile();
+File snapFile2 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(2L));
+snapFile2.createNewFile();
+
+Assert.assertTrue(logFile1.exists());
+Assert.assertTrue(logFile2.exists());
+Assert.assertTrue(snapFile1.exists());
+Assert.assertTrue(snapFile2.exists());
+
+String priorAutocreateDirValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
"false");
+FileTxnSnapLog fileTxnSnapLog;
+try {
+fileTxnSnapLog = new FileTxnSnapLog(logDir, snapDir);
+} catch (FileTxnSnapLog.LogdirContentCheckException e) {
+Assert.fail("Should not throw LogdirContentCheckException.");
+} catch (FileTxnSnapLog.SnapdirContentCheckException e) {
+Assert.fail("Should not throw SnapdirContentCheckException.");
+} finally {
+if (priorAutocreateDirValue == null) {
+
System.clearProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+} else {
+
System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
priorAutocreateDirValue);
+}
+}
+}
+
+@Test
+public void testDirCheckWithSameLogAndSnapDirs() throws IOException {
--- End diff --

I don't repeat my previous comments, all of them applies to the rest of 
test cases.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334335#comment-16334335
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162945532
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -294,5 +297,25 @@ public int compare(File o1, File o2) {
 Collections.sort(filelist, new DataDirFileComparator(prefix, 
ascending));
 return filelist;
 }
+
+/**
+ * Returns true if file is a log file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isLogFile(File file) {
+return file.getName().startsWith(LOG_FILE_PREFIX);
--- End diff --

It might be safer to check here with the dot (".") included in the prefix.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334332#comment-16334332
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162947464
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
+
+// transaction log files in log dir - correct
+File logFile1 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(1L));
+logFile1.createNewFile();
+File logFile2 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(2L));
+logFile2.createNewFile();
+
+// snapshot files in snap dir - correct
+File snapFile1 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(1L));
+snapFile1.createNewFile();
+File snapFile2 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(2L));
+snapFile2.createNewFile();
+
+Assert.assertTrue(logFile1.exists());
+Assert.assertTrue(logFile2.exists());
+Assert.assertTrue(snapFile1.exists());
+Assert.assertTrue(snapFile2.exists());
+
+String priorAutocreateDirValue = 
System.getProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE);
+System.setProperty(FileTxnSnapLog.ZOOKEEPER_DATADIR_AUTOCREATE, 
"false");
+FileTxnSnapLog fileTxnSnapLog;
+try {
+fileTxnSnapLog = new FileTxnSnapLog(logDir, snapDir);
+} catch (FileTxnSnapLog.LogdirContentCheckException e) {
+Assert.fail("Should not throw LogdirContentCheckException.");
+} catch (FileTxnSnapLog.SnapdirContentCheckException e) {
--- End diff --

These 2 catches can be grouped by catching all Exceptions.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334328#comment-16334328
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162929390
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
+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();
+if(files != null) {
+boolean hasSnapshotFiles = false;
+for (File file : files) {
+if(Util.isSnapshotFile(file)){
+hasSnapshotFiles = true;
+break;
+}
+}
+if (hasSnapshotFiles) {
+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();
--- End diff --

Same here. Get files with fileNameFilter and if it returns non-empty array, 
throw exception.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334331#comment-16334331
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162947080
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
+
+// transaction log files in log dir - correct
+File logFile1 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(1L));
+logFile1.createNewFile();
+File logFile2 = new File(logVersionDir.getPath() +File.separator + 
Util.makeLogName(2L));
+logFile2.createNewFile();
+
+// snapshot files in snap dir - correct
+File snapFile1 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(1L));
+snapFile1.createNewFile();
+File snapFile2 = new File(snapVersionDir.getPath() +File.separator 
+ Util.makeSnapshotName(2L));
+snapFile2.createNewFile();
+
+Assert.assertTrue(logFile1.exists());
+Assert.assertTrue(logFile2.exists());
+Assert.assertTrue(snapFile1.exists());
+Assert.assertTrue(snapFile2.exists());
--- End diff --

Again, I believe these checks are redundant. createNewFile() will always 
successfully create the file or throws exception.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334336#comment-16334336
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162946062
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
--- End diff --

This is always true.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334329#comment-16334329
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162945589
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -294,5 +297,25 @@ public int compare(File o1, File o2) {
 Collections.sort(filelist, new DataDirFileComparator(prefix, 
ascending));
 return filelist;
 }
+
+/**
+ * Returns true if file is a log file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isLogFile(File file) {
+return file.getName().startsWith(LOG_FILE_PREFIX);
+}
+
+/**
+ * Returns true if file is a snapshot file.
+ *
+ * @param file
+ * @return
+ */
+public static boolean isSnapshotFile(File file) {
+return file.getName().startsWith(SNAP_FILE_PREFIX);
--- End diff --

Same here with the dot.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334333#comment-16334333
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162945403
  
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/Util.java 
---
@@ -83,7 +86,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 LOG_FILE_PREFIX + "." + Long.toHexString(zxid);
--- End diff --

I wonder if LOG_FILE_PREFIX included the dot, would be better for the 
validation.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334327#comment-16334327
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162946041
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
--- End diff --

This is always true.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334330#comment-16334330
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162929129
  
--- Diff: 
src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java ---
@@ -136,13 +136,51 @@ 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
+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();
--- End diff --

What do you think of using one of the filtered version of listFiles() 
rather than getting all the files in the directory?
For example, using with FilenameFilter:

https://docs.oracle.com/javase/7/docs/api/java/io/File.html#listFiles(java.io.FilenameFilter)


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-22 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334338#comment-16334338
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

Github user anmolnar commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/450#discussion_r162946298
  
--- Diff: 
src/java/test/org/apache/zookeeper/server/persistence/FileTxnSnapLogTest.java 
---
@@ -159,4 +159,222 @@ public void onTxnLoaded(TxnHeader hdr, Record rec) {
 }
 }
 }
+
+@Test
+public void testDirCheckWithCorrectFiles() throws IOException {
+File tmpDir = ClientBase.createEmptyTestDir();
+File logDir = new File(tmpDir, "logdir");
+File snapDir = new File(tmpDir, "snapdir");
+File logVersionDir = new File(logDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+File snapVersionDir = new File(snapDir, FileTxnSnapLog.version +  
FileTxnSnapLog.VERSION);
+
+if (!logVersionDir.exists()) {
+logVersionDir.mkdirs();
+}
+if (!snapVersionDir.exists()) {
+snapVersionDir.mkdirs();
+}
+
+Assert.assertTrue(logVersionDir.exists());
+Assert.assertTrue(snapVersionDir.exists());
--- End diff --

These assert will never fail. mkdirs() throws exception if the directory 
cannot be created.


> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ZOOKEEPER-2967) Add check to validate dataDir and dataLogDir parameters at startup

2018-01-19 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16333027#comment-16333027
 ] 

ASF GitHub Bot commented on ZOOKEEPER-2967:
---

GitHub user mfenes opened a pull request:

https://github.com/apache/zookeeper/pull/450

ZOOKEEPER-2967: Add check to validate dataDir and dataLogDir paramete…

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)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/mfenes/zookeeper ZOOKEEPER-2967

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/450.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 #450


commit 81c026fe8498107f42e9d3599a515c8817f8bf02
Author: Mark Fenes 
Date:   2018-01-19T23:16:07Z

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




> Add check to validate dataDir and dataLogDir parameters at startup
> --
>
> Key: ZOOKEEPER-2967
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2967
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.4.11
>Reporter: Andor Molnar
>Assignee: Mark Fenes
>Priority: Major
>  Labels: startup, validation
> Fix For: 3.5.4, 3.6.0, 3.4.12
>
>
> According to  -ZOOKEEPER-2960- we should at a startup check to validate that 
> dataDir and dataLogDir parameters are set correctly.
> Perhaps we should introduce a check of some kind? If datalogdir is different 
> that datadir and snapshots exist in datalogdir we throw an exception and quit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

52 matches

Mail list logo