You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Flavio Junqueira <fp...@apache.org> on 2016/09/19 08:28:30 UTC

Re: zookeeper git commit: ZOOKEEPER-2579: ZooKeeper server should verify that dataDir and snapDir are writeable before starting (Abraham Fine via phunt)

There is something wrong with the "reply to" of the commit messages.

-Flavio

> On 18 Sep 2016, at 22:30, phunt@apache.org wrote:
> 
> Repository: zookeeper
> Updated Branches:
>  refs/heads/branch-3.5 12f6f6f08 -> ebacab008
> 
> 
> ZOOKEEPER-2579: ZooKeeper server should verify that dataDir and snapDir are writeable before starting (Abraham Fine via phunt)
> 
> Change-Id: I51aacf942bf6daa53d480caa5ca122c229162ec3
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
> Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/ebacab00
> Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/ebacab00
> Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/ebacab00
> 
> Branch: refs/heads/branch-3.5
> Commit: ebacab00875ac8ce4700e9a14c12219f2de00cd5
> Parents: 12f6f6f
> Author: Patrick Hunt <ph...@apache.org>
> Authored: Sun Sep 18 14:26:26 2016 -0700
> Committer: Patrick Hunt <ph...@apache.org>
> Committed: Sun Sep 18 14:26:26 2016 -0700
> 
> ----------------------------------------------------------------------
> CHANGES.txt                                     |  3 +
> .../server/persistence/FileTxnSnapLog.java      |  8 ++
> .../server/ZooKeeperServerMainTest.java         | 94 ++++++++++++++++++--
> 3 files changed, 96 insertions(+), 9 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/zookeeper/blob/ebacab00/CHANGES.txt
> ----------------------------------------------------------------------
> diff --git a/CHANGES.txt b/CHANGES.txt
> index 2a1c328..1b67237 100644
> --- a/CHANGES.txt
> +++ b/CHANGES.txt
> @@ -62,6 +62,9 @@ BUGFIXES:
>   ZOOKEEPER-2484: Flaky Test: org.apache.zookeeper.test.LoadFromLogTest.testLoadFailure
>   (Michael Han via phunt)
> 
> +  ZOOKEEPER-2579: ZooKeeper server should verify that dataDir and
> +  snapDir are writeable before starting (Abraham Fine via phunt)
> +
> IMPROVEMENTS:
> 
>   ZOOKEEPER-2505: Use shared library instead of static library in C
> 
> http://git-wip-us.apache.org/repos/asf/zookeeper/blob/ebacab00/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
> ----------------------------------------------------------------------
> diff --git a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
> index c9a06d7..9a34bd1 100644
> --- a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
> +++ b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
> @@ -106,6 +106,10 @@ public class FileTxnSnapLog {
>                         + this.dataDir);
>             }
>         }
> +        if (!this.dataDir.canWrite()) {
> +            throw new DatadirException("Cannot write to data directory " + this.dataDir);
> +        }
> +
>         if (!this.snapDir.exists()) {
>             // by default create this directory, but otherwise complain instead
>             // See ZOOKEEPER-1161 for more details
> @@ -122,6 +126,10 @@ public class FileTxnSnapLog {
>                         + this.snapDir);
>             }
>         }
> +        if (!this.snapDir.canWrite()) {
> +            throw new DatadirException("Cannot write to snap directory " + this.snapDir);
> +        }
> +
>         txnLog = new FileTxnLog(this.dataDir);
>         snapLog = new FileSnap(this.snapDir);
>     }
> 
> http://git-wip-us.apache.org/repos/asf/zookeeper/blob/ebacab00/src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
> ----------------------------------------------------------------------
> diff --git a/src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java b/src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
> index 1fcd865..1030209 100644
> --- a/src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
> +++ b/src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
> @@ -59,11 +59,18 @@ public class ZooKeeperServerMainTest extends ZKTestCase implements Watcher {
>         final File confFile;
>         final TestZKSMain main;
>         final File tmpDir;
> +        final File dataDir;
> +        final File logDir;
> 
>         public MainThread(int clientPort, boolean preCreateDirs, String configs)
>                 throws IOException {
> +            this(clientPort, preCreateDirs, ClientBase.createTmpDir(), configs);
> +        }
> +
> +        public MainThread(int clientPort, boolean preCreateDirs, File tmpDir, String configs)
> +                throws IOException {
>             super("Standalone server with clientPort:" + clientPort);
> -            tmpDir = ClientBase.createTmpDir();
> +            this.tmpDir = tmpDir;
>             confFile = new File(tmpDir, "zoo.cfg");
> 
>             FileWriter fwriter = new FileWriter(confFile);
> @@ -74,20 +81,21 @@ public class ZooKeeperServerMainTest extends ZKTestCase implements Watcher {
>                 fwriter.write(configs);
>             }
> 
> -            File dataDir = new File(tmpDir, "data");
> -            String dir = dataDir.toString();
> -            String dirLog = dataDir.toString() + "_txnlog";
> +            dataDir = new File(this.tmpDir, "data");
> +            logDir = new File(dataDir.toString() + "_txnlog");
>             if (preCreateDirs) {
>                 if (!dataDir.mkdir()) {
>                     throw new IOException("unable to mkdir " + dataDir);
>                 }
> -                dirLog = dataDir.toString();
> +                if (!logDir.mkdir()) {
> +                    throw new IOException("unable to mkdir " + logDir);
> +                }
>             }
> 
> -            dir = PathUtils.normalizeFileSystemPath(dir);
> -            dirLog = PathUtils.normalizeFileSystemPath(dirLog);
> -            fwriter.write("dataDir=" + dir + "\n");
> -            fwriter.write("dataLogDir=" + dirLog + "\n");
> +            String normalizedDataDir = PathUtils.normalizeFileSystemPath(dataDir.toString());
> +            String normalizedLogDir = PathUtils.normalizeFileSystemPath(logDir.toString());
> +            fwriter.write("dataDir=" + normalizedDataDir + "\n");
> +            fwriter.write("dataLogDir=" + normalizedLogDir + "\n");
>             fwriter.write("clientPort=" + clientPort + "\n");
>             fwriter.flush();
>             fwriter.close();
> @@ -194,6 +202,74 @@ public class ZooKeeperServerMainTest extends ZKTestCase implements Watcher {
>         main.deleteDirs();
>     }
> 
> +    @Test(timeout = 30000)
> +    public void testReadOnlySnapshotDir() throws Exception {
> +        ClientBase.setupTestEnv();
> +        final int CLIENT_PORT = PortAssignment.unique();
> +
> +        // Start up the ZK server to automatically create the necessary directories
> +        // and capture the directory where data is stored
> +        MainThread main = new MainThread(CLIENT_PORT, true, null);
> +        File tmpDir = main.tmpDir;
> +        main.start();
> +        Assert.assertTrue("waiting for server being up", ClientBase
> +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> +                        CONNECTION_TIMEOUT / 2));
> +        main.shutdown();
> +
> +        // Make the snapshot directory read only
> +        File snapDir = new File(main.dataDir, FileTxnSnapLog.version + FileTxnSnapLog.VERSION);
> +        snapDir.setWritable(false);
> +
> +        // Restart ZK and observe a failure
> +        main = new MainThread(CLIENT_PORT, false, tmpDir, null);
> +        main.start();
> +
> +        Assert.assertFalse("waiting for server being up", ClientBase
> +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> +                        CONNECTION_TIMEOUT / 2));
> +
> +        main.shutdown();
> +
> +        snapDir.setWritable(true);
> +
> +        main.deleteDirs();
> +    }
> +
> +    @Test(timeout = 30000)
> +    public void testReadOnlyTxnLogDir() throws Exception {
> +        ClientBase.setupTestEnv();
> +        final int CLIENT_PORT = PortAssignment.unique();
> +
> +        // Start up the ZK server to automatically create the necessary directories
> +        // and capture the directory where data is stored
> +        MainThread main = new MainThread(CLIENT_PORT, true, null);
> +        File tmpDir = main.tmpDir;
> +        main.start();
> +        Assert.assertTrue("waiting for server being up", ClientBase
> +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> +                        CONNECTION_TIMEOUT / 2));
> +        main.shutdown();
> +
> +        // Make the transaction log directory read only
> +        File logDir = new File(main.logDir, FileTxnSnapLog.version + FileTxnSnapLog.VERSION);
> +        logDir.setWritable(false);
> +
> +        // Restart ZK and observe a failure
> +        main = new MainThread(CLIENT_PORT, false, tmpDir, null);
> +        main.start();
> +
> +        Assert.assertFalse("waiting for server being up", ClientBase
> +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> +                        CONNECTION_TIMEOUT / 2));
> +
> +        main.shutdown();
> +
> +        logDir.setWritable(true);
> +
> +        main.deleteDirs();
> +    }
> +
>     /**
>      * Verify the ability to start a standalone server instance.
>      */
> 


Re: zookeeper git commit: ZOOKEEPER-2579: ZooKeeper server should verify that dataDir and snapDir are writeable before starting (Abraham Fine via phunt)

Posted by Patrick Hunt <ph...@apache.org>.
Weird. I reported this to infra:
https://issues.apache.org/jira/browse/INFRA-12631

Patrick

On Mon, Sep 19, 2016 at 1:28 AM, Flavio Junqueira <fp...@apache.org> wrote:

> There is something wrong with the "reply to" of the commit messages.
>
> -Flavio
>
> > On 18 Sep 2016, at 22:30, phunt@apache.org wrote:
> >
> > Repository: zookeeper
> > Updated Branches:
> >  refs/heads/branch-3.5 12f6f6f08 -> ebacab008
> >
> >
> > ZOOKEEPER-2579: ZooKeeper server should verify that dataDir and snapDir
> are writeable before starting (Abraham Fine via phunt)
> >
> > Change-Id: I51aacf942bf6daa53d480caa5ca122c229162ec3
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/ebacab00
> > Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/ebacab00
> > Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/ebacab00
> >
> > Branch: refs/heads/branch-3.5
> > Commit: ebacab00875ac8ce4700e9a14c12219f2de00cd5
> > Parents: 12f6f6f
> > Author: Patrick Hunt <ph...@apache.org>
> > Authored: Sun Sep 18 14:26:26 2016 -0700
> > Committer: Patrick Hunt <ph...@apache.org>
> > Committed: Sun Sep 18 14:26:26 2016 -0700
> >
> > ----------------------------------------------------------------------
> > CHANGES.txt                                     |  3 +
> > .../server/persistence/FileTxnSnapLog.java      |  8 ++
> > .../server/ZooKeeperServerMainTest.java         | 94
> ++++++++++++++++++--
> > 3 files changed, 96 insertions(+), 9 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> > http://git-wip-us.apache.org/repos/asf/zookeeper/blob/
> ebacab00/CHANGES.txt
> > ----------------------------------------------------------------------
> > diff --git a/CHANGES.txt b/CHANGES.txt
> > index 2a1c328..1b67237 100644
> > --- a/CHANGES.txt
> > +++ b/CHANGES.txt
> > @@ -62,6 +62,9 @@ BUGFIXES:
> >   ZOOKEEPER-2484: Flaky Test: org.apache.zookeeper.test.LoadFromLogTest.
> testLoadFailure
> >   (Michael Han via phunt)
> >
> > +  ZOOKEEPER-2579: ZooKeeper server should verify that dataDir and
> > +  snapDir are writeable before starting (Abraham Fine via phunt)
> > +
> > IMPROVEMENTS:
> >
> >   ZOOKEEPER-2505: Use shared library instead of static library in C
> >
> > http://git-wip-us.apache.org/repos/asf/zookeeper/blob/
> ebacab00/src/java/main/org/apache/zookeeper/server/
> persistence/FileTxnSnapLog.java
> > ----------------------------------------------------------------------
> > diff --git a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
> b/src/java/main/org/apache/zookeeper/server/persistence/
> FileTxnSnapLog.java
> > index c9a06d7..9a34bd1 100644
> > --- a/src/java/main/org/apache/zookeeper/server/persistence/
> FileTxnSnapLog.java
> > +++ b/src/java/main/org/apache/zookeeper/server/persistence/
> FileTxnSnapLog.java
> > @@ -106,6 +106,10 @@ public class FileTxnSnapLog {
> >                         + this.dataDir);
> >             }
> >         }
> > +        if (!this.dataDir.canWrite()) {
> > +            throw new DatadirException("Cannot write to data directory
> " + this.dataDir);
> > +        }
> > +
> >         if (!this.snapDir.exists()) {
> >             // by default create this directory, but otherwise complain
> instead
> >             // See ZOOKEEPER-1161 for more details
> > @@ -122,6 +126,10 @@ public class FileTxnSnapLog {
> >                         + this.snapDir);
> >             }
> >         }
> > +        if (!this.snapDir.canWrite()) {
> > +            throw new DatadirException("Cannot write to snap directory
> " + this.snapDir);
> > +        }
> > +
> >         txnLog = new FileTxnLog(this.dataDir);
> >         snapLog = new FileSnap(this.snapDir);
> >     }
> >
> > http://git-wip-us.apache.org/repos/asf/zookeeper/blob/
> ebacab00/src/java/test/org/apache/zookeeper/server/
> ZooKeeperServerMainTest.java
> > ----------------------------------------------------------------------
> > diff --git a/src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
> b/src/java/test/org/apache/zookeeper/server/ZooKeeperServerMainTest.java
> > index 1fcd865..1030209 100644
> > --- a/src/java/test/org/apache/zookeeper/server/
> ZooKeeperServerMainTest.java
> > +++ b/src/java/test/org/apache/zookeeper/server/
> ZooKeeperServerMainTest.java
> > @@ -59,11 +59,18 @@ public class ZooKeeperServerMainTest extends
> ZKTestCase implements Watcher {
> >         final File confFile;
> >         final TestZKSMain main;
> >         final File tmpDir;
> > +        final File dataDir;
> > +        final File logDir;
> >
> >         public MainThread(int clientPort, boolean preCreateDirs, String
> configs)
> >                 throws IOException {
> > +            this(clientPort, preCreateDirs, ClientBase.createTmpDir(),
> configs);
> > +        }
> > +
> > +        public MainThread(int clientPort, boolean preCreateDirs, File
> tmpDir, String configs)
> > +                throws IOException {
> >             super("Standalone server with clientPort:" + clientPort);
> > -            tmpDir = ClientBase.createTmpDir();
> > +            this.tmpDir = tmpDir;
> >             confFile = new File(tmpDir, "zoo.cfg");
> >
> >             FileWriter fwriter = new FileWriter(confFile);
> > @@ -74,20 +81,21 @@ public class ZooKeeperServerMainTest extends
> ZKTestCase implements Watcher {
> >                 fwriter.write(configs);
> >             }
> >
> > -            File dataDir = new File(tmpDir, "data");
> > -            String dir = dataDir.toString();
> > -            String dirLog = dataDir.toString() + "_txnlog";
> > +            dataDir = new File(this.tmpDir, "data");
> > +            logDir = new File(dataDir.toString() + "_txnlog");
> >             if (preCreateDirs) {
> >                 if (!dataDir.mkdir()) {
> >                     throw new IOException("unable to mkdir " + dataDir);
> >                 }
> > -                dirLog = dataDir.toString();
> > +                if (!logDir.mkdir()) {
> > +                    throw new IOException("unable to mkdir " + logDir);
> > +                }
> >             }
> >
> > -            dir = PathUtils.normalizeFileSystemPath(dir);
> > -            dirLog = PathUtils.normalizeFileSystemPath(dirLog);
> > -            fwriter.write("dataDir=" + dir + "\n");
> > -            fwriter.write("dataLogDir=" + dirLog + "\n");
> > +            String normalizedDataDir = PathUtils.
> normalizeFileSystemPath(dataDir.toString());
> > +            String normalizedLogDir = PathUtils.
> normalizeFileSystemPath(logDir.toString());
> > +            fwriter.write("dataDir=" + normalizedDataDir + "\n");
> > +            fwriter.write("dataLogDir=" + normalizedLogDir + "\n");
> >             fwriter.write("clientPort=" + clientPort + "\n");
> >             fwriter.flush();
> >             fwriter.close();
> > @@ -194,6 +202,74 @@ public class ZooKeeperServerMainTest extends
> ZKTestCase implements Watcher {
> >         main.deleteDirs();
> >     }
> >
> > +    @Test(timeout = 30000)
> > +    public void testReadOnlySnapshotDir() throws Exception {
> > +        ClientBase.setupTestEnv();
> > +        final int CLIENT_PORT = PortAssignment.unique();
> > +
> > +        // Start up the ZK server to automatically create the necessary
> directories
> > +        // and capture the directory where data is stored
> > +        MainThread main = new MainThread(CLIENT_PORT, true, null);
> > +        File tmpDir = main.tmpDir;
> > +        main.start();
> > +        Assert.assertTrue("waiting for server being up", ClientBase
> > +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> > +                        CONNECTION_TIMEOUT / 2));
> > +        main.shutdown();
> > +
> > +        // Make the snapshot directory read only
> > +        File snapDir = new File(main.dataDir, FileTxnSnapLog.version +
> FileTxnSnapLog.VERSION);
> > +        snapDir.setWritable(false);
> > +
> > +        // Restart ZK and observe a failure
> > +        main = new MainThread(CLIENT_PORT, false, tmpDir, null);
> > +        main.start();
> > +
> > +        Assert.assertFalse("waiting for server being up", ClientBase
> > +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> > +                        CONNECTION_TIMEOUT / 2));
> > +
> > +        main.shutdown();
> > +
> > +        snapDir.setWritable(true);
> > +
> > +        main.deleteDirs();
> > +    }
> > +
> > +    @Test(timeout = 30000)
> > +    public void testReadOnlyTxnLogDir() throws Exception {
> > +        ClientBase.setupTestEnv();
> > +        final int CLIENT_PORT = PortAssignment.unique();
> > +
> > +        // Start up the ZK server to automatically create the necessary
> directories
> > +        // and capture the directory where data is stored
> > +        MainThread main = new MainThread(CLIENT_PORT, true, null);
> > +        File tmpDir = main.tmpDir;
> > +        main.start();
> > +        Assert.assertTrue("waiting for server being up", ClientBase
> > +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> > +                        CONNECTION_TIMEOUT / 2));
> > +        main.shutdown();
> > +
> > +        // Make the transaction log directory read only
> > +        File logDir = new File(main.logDir, FileTxnSnapLog.version +
> FileTxnSnapLog.VERSION);
> > +        logDir.setWritable(false);
> > +
> > +        // Restart ZK and observe a failure
> > +        main = new MainThread(CLIENT_PORT, false, tmpDir, null);
> > +        main.start();
> > +
> > +        Assert.assertFalse("waiting for server being up", ClientBase
> > +                .waitForServerUp("127.0.0.1:" + CLIENT_PORT,
> > +                        CONNECTION_TIMEOUT / 2));
> > +
> > +        main.shutdown();
> > +
> > +        logDir.setWritable(true);
> > +
> > +        main.deleteDirs();
> > +    }
> > +
> >     /**
> >      * Verify the ability to start a standalone server instance.
> >      */
> >
>
>