Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: elserj@apache.org
To: commits@accumulo.apache.org
Date: Fri, 18 Oct 2013 02:02:18 -0000
Message-Id: <c79da3ecb3714857b668aad6e66dd2a8@git.apache.org>
Subject: [1/2] git commit: ACCUMULO-1637 Add tests for append/sync conf check

Updated Branches:
  refs/heads/master e26fc0bff -> e498ebb80


ACCUMULO-1637 Add tests for append/sync conf check

Added in a few tests, given a hadoop-1.0.4 dependency, that make sure we
fail when appropriate. Had to change the access control on the method in
TabletServer as well as throwing an exception instead of directly
calling System.exit() to make a more easily testable method.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/09876282
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/09876282
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/09876282

Branch: refs/heads/master
Commit: 0987628299165740230de132f85b3c63789ad584
Parents: 18bcd14
Author: Josh Elser <elserj@apache.org>
Authored: Thu Oct 17 21:38:14 2013 -0400
Committer: Josh Elser <elserj@apache.org>
Committed: Thu Oct 17 21:38:14 2013 -0400

----------------------------------------------------------------------
 .../server/tabletserver/TabletServer.java       |  15 +--
 .../tabletserver/TabletServerSyncCheckTest.java | 105 +++++++++++++++++++
 2 files changed, 114 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/09876282/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java
----------------------------------------------------------------------
diff --git a/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java b/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java
index 7ad106e..eba96ae 100644
--- a/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java
+++ b/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java
@@ -3242,7 +3242,7 @@ public class TabletServer extends AbstractMetricsImpl implements org.apache.accu
     }
   }
   
-  private static void ensureHdfsSyncIsEnabled(FileSystem fs) {
+  protected static void ensureHdfsSyncIsEnabled(FileSystem fs) {
     if (fs instanceof DistributedFileSystem) {
       final String DFS_DURABLE_SYNC = "dfs.durable.sync", DFS_SUPPORT_APPEND = "dfs.support.append";
       final String ticketMessage = "See ACCUMULO-623 and ACCUMULO-1637 for more details.";
@@ -3258,22 +3258,25 @@ public class TabletServer extends AbstractMetricsImpl implements org.apache.accu
         if (!dfsSupportAppendDefaultValue) {
           // See if the user did the correct override
           if (!fs.getConf().getBoolean(DFS_SUPPORT_APPEND, false)) {
-            log.fatal("Accumulo requires that dfs.support.append to true. " + ticketMessage);
-            System.exit(-1);
+            String msg = "Accumulo requires that dfs.support.append to true. " + ticketMessage;
+            log.fatal(msg);
+            throw new RuntimeException(msg);
           }
         }
       } catch (NoSuchFieldException e) {
         // If we can't find DFSConfigKeys.DFS_SUPPORT_APPEND_DEFAULT, the user is running
         // 1.1.x or 1.2.x. This is ok, though, as, by default, these versions have append/sync enabled.
       } catch (Exception e) {
-        log.warn("Error while checking for " + DFS_SUPPORT_APPEND + ". The user should ensure that Hadoop is configured to properly supports append and sync. " + ticketMessage, e);
+        log.warn("Error while checking for " + DFS_SUPPORT_APPEND + ". The user should ensure that Hadoop is configured to properly supports append and sync. "
+            + ticketMessage, e);
       }
       
       // If either of these parameters are configured to be false, fail.
       // This is a sign that someone is writing bad configuration.
       if (!fs.getConf().getBoolean(DFS_SUPPORT_APPEND, true) || !fs.getConf().getBoolean(DFS_DURABLE_SYNC, true)) {
-        log.fatal("Accumulo requires that " + DFS_SUPPORT_APPEND + " and " + DFS_DURABLE_SYNC + " not be configured as false. " + ticketMessage);
-        System.exit(-1);
+        String msg = "Accumulo requires that " + DFS_SUPPORT_APPEND + " and " + DFS_DURABLE_SYNC + " not be configured as false. " + ticketMessage;
+        log.fatal(msg);
+        throw new RuntimeException(msg);
       }
       
       try {

http://git-wip-us.apache.org/repos/asf/accumulo/blob/09876282/server/src/test/java/org/apache/accumulo/server/tabletserver/TabletServerSyncCheckTest.java
----------------------------------------------------------------------
diff --git a/server/src/test/java/org/apache/accumulo/server/tabletserver/TabletServerSyncCheckTest.java b/server/src/test/java/org/apache/accumulo/server/tabletserver/TabletServerSyncCheckTest.java
new file mode 100644
index 0000000..6443b7e
--- /dev/null
+++ b/server/src/test/java/org/apache/accumulo/server/tabletserver/TabletServerSyncCheckTest.java
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.accumulo.server.tabletserver;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.hdfs.DistributedFileSystem;
+import org.junit.Test;
+
+public class TabletServerSyncCheckTest {
+  private static final String DFS_DURABLE_SYNC = "dfs.durable.sync", DFS_SUPPORT_APPEND = "dfs.support.append";
+  
+  @Test(expected = RuntimeException.class)
+  public void testFailureOnExplicitSyncFalseConf() {
+    Configuration conf = new Configuration();
+    conf.set(DFS_DURABLE_SYNC, "false");
+    
+    FileSystem fs = new TestFileSystem(conf);
+    
+    TabletServer.ensureHdfsSyncIsEnabled(fs);
+  }
+  
+  @Test(expected = RuntimeException.class)
+  public void testFailureOnExplicitAppendFalseConf() {
+    Configuration conf = new Configuration();
+    conf.set(DFS_SUPPORT_APPEND, "false");
+    
+    FileSystem fs = new TestFileSystem(conf);
+    
+    TabletServer.ensureHdfsSyncIsEnabled(fs);
+  }
+  
+  @Test(expected = RuntimeException.class)
+  public void testFailureOnExplicitSyncAndAppendFalseConf() {
+    Configuration conf = new Configuration();
+    conf.set(DFS_SUPPORT_APPEND, "false");
+    conf.set(DFS_DURABLE_SYNC, "false");
+    
+    FileSystem fs = new TestFileSystem(conf);
+    
+    TabletServer.ensureHdfsSyncIsEnabled(fs);
+  }
+  
+  @Test(expected = RuntimeException.class)
+  public void testDefaultHadoopAction() {
+    // We currently depend on Hadoop-1.0.4 in this branch
+    // so this test should throw an exception by default
+    Configuration conf = new Configuration();
+    
+    FileSystem fs = new TestFileSystem(conf);
+    
+    TabletServer.ensureHdfsSyncIsEnabled(fs);
+  }
+  
+  @Test(expected = RuntimeException.class)
+  public void testMissingNecessaryConfiguration() {
+    // We currently depend on Hadoop-1.0.4 in this branch
+    // so this test should throw an exception by default
+    Configuration conf = new Configuration();
+    
+    FileSystem fs = new TestFileSystem(conf);
+    
+    TabletServer.ensureHdfsSyncIsEnabled(fs);
+  }
+  
+  @Test
+  public void testNecessaryConfiguration() {
+    // We currently depend on Hadoop-1.0.4 in this branch
+    // By providing the override, we should not throw an exception
+    Configuration conf = new Configuration();
+    conf.set(DFS_SUPPORT_APPEND, "true");
+    
+    FileSystem fs = new TestFileSystem(conf);
+    
+    TabletServer.ensureHdfsSyncIsEnabled(fs);
+  }
+  
+  private class TestFileSystem extends DistributedFileSystem {
+    protected final Configuration conf;
+    
+    public TestFileSystem(Configuration conf) {
+      this.conf = conf;
+    }
+    
+    @Override
+    public Configuration getConf() {
+      return conf;
+    }
+    
+  }
+}