accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bhava...@apache.org
Subject [2/8] git commit: ACCUMULO-3012 Improve DefaultConfiguration
Date Wed, 13 Aug 2014 14:48:28 GMT
ACCUMULO-3012 Improve DefaultConfiguration

* The map of default properties created during the iterator() method each time is now
  static and immutable.
* A unit test is added.
* Accompanying changes were made to ConfigSanityCheck:
  - The validate method takes in an Iterable of entries instead of a Configuration.
  - Validation failure throws a new SanityCheckException class instead of RuntimeException.
  - A unit test is added.


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

Branch: refs/heads/1.6.1-SNAPSHOT
Commit: 7136996a19406a7dfc5c2f2c5861cae95f68badc
Parents: c4d46bb
Author: Bill Havanki <bhavanki@cloudera.com>
Authored: Thu Jul 24 09:36:04 2014 -0400
Committer: Bill Havanki <bhavanki@cloudera.com>
Committed: Wed Aug 13 09:30:04 2014 -0400

----------------------------------------------------------------------
 .../accumulo/core/conf/ConfigSanityCheck.java   | 44 +++++++++--
 .../core/conf/DefaultConfiguration.java         | 41 ++++++----
 .../core/conf/ConfigSanityCheckTest.java        | 79 ++++++++++++++++++++
 .../core/conf/DefaultConfigurationTest.java     | 47 ++++++++++++
 4 files changed, 187 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/7136996a/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
index 6724670..abec58b 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigSanityCheck.java
@@ -20,13 +20,24 @@ import java.util.Map.Entry;
 
 import org.apache.log4j.Logger;
 
+/**
+ * A utility class for checking over configuration entries.
+ */
 public class ConfigSanityCheck {
   
   private static final Logger log = Logger.getLogger(ConfigSanityCheck.class);
   private static final String PREFIX = "BAD CONFIG ";
   
-  public static void validate(AccumuloConfiguration acuconf) {
-    for (Entry<String,String> entry : acuconf) {
+  /**
+   * Validates the given configuration entries.
+   *
+   * @param entries iterable through configuration keys and values
+   * @throws SanityCheckException if a fatal configuration error is found
+   */
+  public static void validate(Iterable<Entry<String,String>> entries) {
+    String instanceZkTimeoutKey = Property.INSTANCE_ZK_TIMEOUT.getKey();
+    String instanceZkTimeoutValue = null;
+    for (Entry<String,String> entry : entries) {
       String key = entry.getKey();
       String value = entry.getValue();
       Property prop = Property.getPropertyByKey(entry.getKey());
@@ -38,9 +49,16 @@ public class ConfigSanityCheck {
         fatal(PREFIX + "incomplete property key (" + key + ")");
       else if (!prop.getType().isValidFormat(value))
         fatal(PREFIX + "improperly formatted value for key (" + key + ", type=" + prop.getType()
+ ")");
+
+      if (key.equals(instanceZkTimeoutKey)) {
+        instanceZkTimeoutValue = value;
+      }
+    }
+
+    if (instanceZkTimeoutValue != null) {
+      checkTimeDuration(Property.INSTANCE_ZK_TIMEOUT, instanceZkTimeoutValue,
+                        new CheckTimeDurationBetween(1000, 300000));
     }
-    
-    checkTimeDuration(acuconf, Property.INSTANCE_ZK_TIMEOUT, new CheckTimeDurationBetween(1000,
300000));
   }
   
   private static interface CheckTimeDuration {
@@ -68,9 +86,9 @@ public class ConfigSanityCheck {
     }
   }
   
-  private static void checkTimeDuration(AccumuloConfiguration acuconf, Property prop, CheckTimeDuration
chk) {
+  private static void checkTimeDuration(Property prop, String value, CheckTimeDuration chk)
{
     verifyPropertyTypes(PropertyType.TIMEDURATION, prop);
-    if (!chk.check(acuconf.getTimeInMillis(prop)))
+    if (!chk.check(AccumuloConfiguration.getTimeInMillis(value)))
       fatal(PREFIX + chk.getDescription(prop));
   }
   
@@ -79,9 +97,19 @@ public class ConfigSanityCheck {
       if (prop.getType() != type)
         fatal("Unexpected property type (" + prop.getType() + " != " + type + ")");
   }
-  
+
+  /**
+   * The exception thrown when {@link ConfigSanityCheck#validate(Iterable)} fails.
+   */
+  public static class SanityCheckException extends RuntimeException {
+    /**
+     * Creates a new exception with the given message.
+     */
+    public SanityCheckException(String msg) { super(msg); }
+  }
+
   private static void fatal(String msg) {
     log.fatal(msg);
-    throw new RuntimeException(msg);
+    throw new SanityCheckException(msg);
   }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/7136996a/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
index d653274..c83c754 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
@@ -23,6 +23,8 @@ import java.io.PrintStream;
 import java.io.UnsupportedEncodingException;
 import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.Collections;
+import java.util.Map;
 import java.util.Map.Entry;
 import java.util.TreeMap;
 
@@ -30,32 +32,40 @@ import org.apache.accumulo.core.Constants;
 import org.apache.log4j.Logger;
 
 public class DefaultConfiguration extends AccumuloConfiguration {
-  private static DefaultConfiguration instance = null;
   private static Logger log = Logger.getLogger(DefaultConfiguration.class);
-  
-  synchronized public static DefaultConfiguration getInstance() {
-    if (instance == null) {
-      instance = new DefaultConfiguration();
-      ConfigSanityCheck.validate(instance);
+
+  private final static Map<String,String> resolvedProps;
+  static {
+    Map<String,String> m = new TreeMap<String,String>();
+    for (Property prop : Property.values()) {
+      if (!prop.isExperimental() && !prop.getType().equals(PropertyType.PREFIX))
{
+        m.put(prop.getKey(), prop.getDefaultValue());
+      }
     }
-    return instance;
+    ConfigSanityCheck.validate(m.entrySet());
+    resolvedProps = Collections.unmodifiableMap(m);
   }
-  
+
+  /**
+   * Gets a default configuration.
+   *
+   * @return default configuration
+   */
+  public static DefaultConfiguration getInstance() {
+    return new DefaultConfiguration();
+  }
+
   @Override
   public String get(Property property) {
     return property.getDefaultValue();
   }
-  
+
   @Override
   public Iterator<Entry<String,String>> iterator() {
-    TreeMap<String,String> entries = new TreeMap<String,String>();
-    for (Property prop : Property.values())
-      if (!prop.isExperimental() && !prop.getType().equals(PropertyType.PREFIX))
-        entries.put(prop.getKey(), prop.getDefaultValue());
-    
-    return entries.entrySet().iterator();
+    return resolvedProps.entrySet().iterator();
   }
   
+
   private static void generateDocumentation(PrintStream doc) {
     // read static content from resources and output
     InputStream data = DefaultConfiguration.class.getResourceAsStream("config.html");
@@ -185,5 +195,4 @@ public class DefaultConfiguration extends AccumuloConfiguration {
       throw new IllegalArgumentException("Usage: " + DefaultConfiguration.class.getName()
+ " --generate-doc <filename>");
     }
   }
-  
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/7136996a/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java b/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java
new file mode 100644
index 0000000..88bbe64
--- /dev/null
+++ b/core/src/test/java/org/apache/accumulo/core/conf/ConfigSanityCheckTest.java
@@ -0,0 +1,79 @@
+/*
+ * 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.core.conf;
+
+import java.util.Map;
+import org.apache.accumulo.core.conf.ConfigSanityCheck.SanityCheckException;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ConfigSanityCheckTest {
+  private Map<String,String> m;
+
+  @Before
+  public void setUp() {
+    m = new java.util.HashMap<String,String>();
+  }
+
+  @Test
+  public void testPass() {
+    m.put(Property.MASTER_CLIENTPORT.getKey(), "9999");
+    m.put(Property.MASTER_TABLET_BALANCER.getKey(), "org.apache.accumulo.server.master.balancer.TableLoadBalancer");
+    m.put(Property.MASTER_RECOVERY_MAXAGE.getKey(), "60m");
+    m.put(Property.MASTER_BULK_RETRIES.getKey(), "3");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+
+  @Test
+  public void testPass_Empty() {
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+
+  @Test
+  public void testPass_UnrecognizedValidProperty() {
+    m.put(Property.MASTER_CLIENTPORT.getKey(), "9999");
+    m.put(Property.MASTER_PREFIX.getKey() + "something", "abcdefg");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+
+  @Test
+  public void testPass_UnrecognizedProperty() {
+    m.put(Property.MASTER_CLIENTPORT.getKey(), "9999");
+    m.put("invalid.prefix.value", "abcdefg");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+
+  @Test(expected = SanityCheckException.class)
+  public void testFail_Prefix() {
+    m.put(Property.MASTER_CLIENTPORT.getKey(), "9999");
+    m.put(Property.MASTER_PREFIX.getKey(), "oops");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+
+  @Test(expected = SanityCheckException.class)
+  public void testFail_InvalidFormat() {
+    m.put(Property.MASTER_CLIENTPORT.getKey(), "9999");
+    m.put(Property.MASTER_RECOVERY_MAXAGE.getKey(), "60breem");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+
+  @Test(expected = SanityCheckException.class)
+  public void testFail_InstanceZkTimeoutOutOfRange() {
+    m.put(Property.INSTANCE_ZK_TIMEOUT.getKey(), "10ms");
+    ConfigSanityCheck.validate(m.entrySet());
+  }
+}

http://git-wip-us.apache.org/repos/asf/accumulo/blob/7136996a/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
new file mode 100644
index 0000000..9355fff
--- /dev/null
+++ b/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
@@ -0,0 +1,47 @@
+/*
+ * 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.core.conf;
+
+import java.util.Iterator;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+
+public class DefaultConfigurationTest {
+  private DefaultConfiguration c;
+
+  @Before
+  public void setUp() {
+    c = new DefaultConfiguration();
+  }
+
+  @Test
+  public void testGet() {
+    assertEquals(Property.MASTER_CLIENTPORT.getDefaultValue(), c.get(Property.MASTER_CLIENTPORT));
+  }
+
+  @Test
+  public void testGetProperties() {
+    Iterator<Map.Entry<String,String>> iter = c.iterator();
+    while (iter.hasNext()) {
+      Map.Entry<String,String> e = iter.next();
+      Property p = Property.getPropertyByKey(e.getKey());
+      assertEquals(p.getDefaultValue(), e.getValue());
+    }
+  }
+}


Mime
View raw message