accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] branch 1.7 updated: ACCUMULO-4779 Speedup Property by precomputing and avoiding sync (#366)
Date Thu, 01 Feb 2018 16:38:19 GMT
This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch 1.7
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/1.7 by this push:
     new 1fe3ba1  ACCUMULO-4779 Speedup Property by precomputing and avoiding sync (#366)
1fe3ba1 is described below

commit 1fe3ba12a943e590b89b2979e661e7dc447d0774
Author: Keith Turner <keith@deenlo.com>
AuthorDate: Thu Feb 1 11:38:16 2018 -0500

    ACCUMULO-4779 Speedup Property by precomputing and avoiding sync (#366)
---
 .../org/apache/accumulo/core/conf/Property.java    | 154 +++++++++++++--------
 .../apache/accumulo/core/conf/PropertyTest.java    |  58 ++++++++
 2 files changed, 156 insertions(+), 56 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
index 5a32a83..1733124 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@ -39,6 +39,8 @@ import org.apache.commons.configuration.PropertiesConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+
 public enum Property {
   // Crypto-related properties
   @Experimental
@@ -589,7 +591,16 @@ public enum Property {
 
   ;
 
-  private String key, defaultValue, description;
+  private String key;
+  private String defaultValue;
+  private String computedDefaultValue;
+  private String description;
+  private boolean annotationsComputed = false;
+  private boolean defaultValueComputed = false;
+  private boolean isSensitive;
+  private boolean isDeprecated;
+  private boolean isExperimental;
+  private boolean isInterpolated;
   private PropertyType type;
   private static final Logger log = LoggerFactory.getLogger(Property.class);
 
@@ -629,6 +640,11 @@ public enum Property {
    * @return default value
    */
   public String getDefaultValue() {
+    Preconditions.checkState(defaultValueComputed, "precomputeDefaultValue() must be called
before calling this method");
+    return computedDefaultValue;
+  }
+
+  private void precomputeDefaultValue() {
     String v;
     if (isInterpolated()) {
       PropertiesConfiguration pconf = new PropertiesConfiguration();
@@ -643,7 +659,9 @@ public enum Property {
     }
     if (this.type == PropertyType.ABSOLUTEPATH && !(v.trim().equals("")))
       v = new File(v).getAbsolutePath();
-    return v;
+
+    computedDefaultValue = v;
+    defaultValueComputed = true;
   }
 
   /**
@@ -665,7 +683,8 @@ public enum Property {
   }
 
   private boolean isInterpolated() {
-    return hasAnnotation(Interpolated.class) || hasPrefixWithAnnotation(getKey(), Interpolated.class);
+    Preconditions.checkState(annotationsComputed, "precomputeAnnotations() must be called
before calling this method");
+    return isInterpolated;
   }
 
   /**
@@ -674,7 +693,8 @@ public enum Property {
    * @return true if this property is experimental
    */
   public boolean isExperimental() {
-    return hasAnnotation(Experimental.class) || hasPrefixWithAnnotation(getKey(), Experimental.class);
+    Preconditions.checkState(annotationsComputed, "precomputeAnnotations() must be called
before calling this method");
+    return isExperimental;
   }
 
   /**
@@ -683,21 +703,26 @@ public enum Property {
    * @return true if this property is deprecated
    */
   public boolean isDeprecated() {
-    return hasAnnotation(Deprecated.class) || hasPrefixWithAnnotation(getKey(), Deprecated.class);
+    Preconditions.checkState(annotationsComputed, "precomputeAnnotations() must be called
before calling this method");
+    return isDeprecated;
   }
 
-  private volatile Boolean isSensitive = null;
-
   /**
    * Checks if this property is sensitive.
    *
    * @return true if this property is sensitive
    */
   public boolean isSensitive() {
-    if (isSensitive == null) {
-      isSensitive = hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class);
-    }
-    return isSensitive.booleanValue();
+    Preconditions.checkState(annotationsComputed, "precomputeAnnotations() must be called
before calling this method");
+    return isSensitive;
+  }
+
+  private void precomputeAnnotations() {
+    isSensitive = hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class);
+    isDeprecated = hasAnnotation(Deprecated.class) || hasPrefixWithAnnotation(getKey(), Deprecated.class);
+    isExperimental = hasAnnotation(Experimental.class) || hasPrefixWithAnnotation(getKey(),
Experimental.class);
+    isInterpolated = hasAnnotation(Interpolated.class) || hasPrefixWithAnnotation(getKey(),
Interpolated.class);
+    annotationsComputed = true;
   }
 
   /**
@@ -709,7 +734,20 @@ public enum Property {
    * @return true if property is sensitive
    */
   public static boolean isSensitive(String key) {
-    return hasPrefixWithAnnotation(key, Sensitive.class);
+    Property prop = propertiesByKey.get(key);
+    if (prop != null) {
+      return prop.isSensitive();
+    } else {
+      for (String prefix : validPrefixes) {
+        if (key.startsWith(prefix)) {
+          if (propertiesByKey.get(prefix).isSensitive()) {
+            return true;
+          }
+        }
+      }
+    }
+
+    return false;
   }
 
   private <T extends Annotation> boolean hasAnnotation(Class<T> annotationType)
{
@@ -727,24 +765,21 @@ public enum Property {
   }
 
   private static <T extends Annotation> boolean hasPrefixWithAnnotation(String key,
Class<T> annotationType) {
-    // relies on side-effects of isValidPropertyKey to populate validPrefixes
-    if (isValidPropertyKey(key)) {
-      // check if property exists on its own and has the annotation
-      if (Property.getPropertyByKey(key) != null)
-        return getPropertyByKey(key).hasAnnotation(annotationType);
-      // can't find the property, so check the prefixes
-      boolean prefixHasAnnotation = false;
-      for (String prefix : validPrefixes)
-        if (key.startsWith(prefix))
-          prefixHasAnnotation = prefixHasAnnotation || getPropertyByKey(prefix).hasAnnotation(annotationType);
-      return prefixHasAnnotation;
+    for (String prefix : validPrefixes) {
+      if (key.startsWith(prefix)) {
+        if (propertiesByKey.get(prefix).hasAnnotation(annotationType)) {
+          return true;
+        }
+      }
     }
+
     return false;
   }
 
   private static HashSet<String> validTableProperties = null;
   private static HashSet<String> validProperties = null;
   private static HashSet<String> validPrefixes = null;
+  private static HashMap<String,Property> propertiesByKey = null;
 
   private static boolean isKeyValidlyPrefixed(String key) {
     for (String prefix : validPrefixes) {
@@ -763,20 +798,7 @@ public enum Property {
    *          property key
    * @return true if key is valid (recognized, or has a recognized prefix)
    */
-  public synchronized static boolean isValidPropertyKey(String key) {
-    if (validProperties == null) {
-      validProperties = new HashSet<>();
-      validPrefixes = new HashSet<>();
-
-      for (Property p : Property.values()) {
-        if (p.getType().equals(PropertyType.PREFIX)) {
-          validPrefixes.add(p.getKey());
-        } else {
-          validProperties.add(p.getKey());
-        }
-      }
-    }
-
+  public static boolean isValidPropertyKey(String key) {
     return validProperties.contains(key) || isKeyValidlyPrefixed(key);
   }
 
@@ -789,20 +811,12 @@ public enum Property {
    *          property key
    * @return true if key is valid for a table property (recognized, or has a recognized prefix)
    */
-  public synchronized static boolean isValidTablePropertyKey(String key) {
-    if (validTableProperties == null) {
-      validTableProperties = new HashSet<>();
-      for (Property p : Property.values()) {
-        if (!p.getType().equals(PropertyType.PREFIX) && p.getKey().startsWith(Property.TABLE_PREFIX.getKey()))
{
-          validTableProperties.add(p.getKey());
-        }
-      }
-    }
-
-    return validTableProperties.contains(key) || key.startsWith(Property.TABLE_CONSTRAINT_PREFIX.getKey())
-        || key.startsWith(Property.TABLE_ITERATOR_PREFIX.getKey()) || key.startsWith(Property.TABLE_LOCALITY_GROUP_PREFIX.getKey())
-        || key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey()) || key.startsWith(Property.TABLE_REPLICATION_TARGET.getKey())
-        || key.startsWith(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey());
+  public static boolean isValidTablePropertyKey(String key) {
+    return validTableProperties.contains(key)
+        || (key.startsWith(Property.TABLE_PREFIX.getKey()) && (key.startsWith(Property.TABLE_CONSTRAINT_PREFIX.getKey())
+            || key.startsWith(Property.TABLE_ITERATOR_PREFIX.getKey()) || key.startsWith(Property.TABLE_LOCALITY_GROUP_PREFIX.getKey())
+            || key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey()) || key.startsWith(Property.TABLE_REPLICATION_TARGET.getKey())
|| key
+              .startsWith(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey())));
   }
 
   /**
@@ -838,7 +852,7 @@ public enum Property {
     return key.startsWith(Property.TABLE_PREFIX.getKey()) || key.startsWith(Property.TSERV_PREFIX.getKey())
|| key.startsWith(Property.LOGGER_PREFIX.getKey())
         || key.startsWith(Property.MASTER_PREFIX.getKey()) || key.startsWith(Property.GC_PREFIX.getKey())
         || key.startsWith(Property.MONITOR_PREFIX.getKey() + "banner.") || key.startsWith(VFS_CONTEXT_CLASSPATH_PROPERTY.getKey())
-        || key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey()) || key.startsWith(REPLICATION_PREFIX.getKey());
+        || key.startsWith(REPLICATION_PREFIX.getKey());
   }
 
   /**
@@ -849,10 +863,7 @@ public enum Property {
    * @return property, or null if not found
    */
   public static Property getPropertyByKey(String key) {
-    for (Property prop : Property.values())
-      if (prop.getKey().equals(key))
-        return prop;
-    return null;
+    return propertiesByKey.get(key);
   }
 
   /**
@@ -952,4 +963,35 @@ public enum Property {
     }
     return result;
   }
+
+  static {
+    // Precomputing information here avoids :
+    // * Computing it each time a method is called
+    // * Using synch to compute the first time a method is called
+    propertiesByKey = new HashMap<>();
+    validPrefixes = new HashSet<>();
+    validProperties = new HashSet<>();
+
+    for (Property p : Property.values()) {
+      if (p.getType().equals(PropertyType.PREFIX)) {
+        validPrefixes.add(p.getKey());
+      } else {
+        validProperties.add(p.getKey());
+      }
+      propertiesByKey.put(p.getKey(), p);
+    }
+
+    validTableProperties = new HashSet<>();
+    for (Property p : Property.values()) {
+      if (!p.getType().equals(PropertyType.PREFIX) && p.getKey().startsWith(Property.TABLE_PREFIX.getKey()))
{
+        validTableProperties.add(p.getKey());
+      }
+    }
+
+    // order is very important here the following code relies on the maps and sets populated
above
+    for (Property p : Property.values()) {
+      p.precomputeAnnotations();
+      p.precomputeDefaultValue();
+    }
+  }
 }
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
index 6848ee4..d25c8c2 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
@@ -19,6 +19,7 @@ package org.apache.accumulo.core.conf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
@@ -152,4 +153,61 @@ public class PropertyTest {
   public void testGCDeadServerWaitSecond() {
     assertEquals("1h", Property.GC_WAL_DEAD_SERVER_WAIT.getDefaultValue());
   }
+
+  @SuppressWarnings("deprecation")
+  private Property getDeprecatedProperty() {
+    return Property.INSTANCE_DFS_DIR;
+  }
+
+  @Test
+  public void testAnnotations() {
+    assertTrue(Property.TABLE_VOLUME_CHOOSER.isExperimental());
+    assertFalse(Property.LOGGER_DIR.isExperimental());
+
+    assertTrue(Property.INSTANCE_SECRET.isSensitive());
+    assertFalse(Property.INSTANCE_VOLUMES.isSensitive());
+
+    assertTrue(getDeprecatedProperty().isDeprecated());
+    assertFalse(Property.INSTANCE_VOLUMES_REPLACEMENTS.isDeprecated());
+
+  }
+
+  @Test
+  public void testGetPropertyByKey() {
+    for (Property prop : Property.values()) {
+      assertSame(prop, Property.getPropertyByKey(prop.getKey()));
+    }
+  }
+
+  @Test
+  public void testIsValidPropertyKey() {
+    for (Property prop : Property.values()) {
+      assertTrue(Property.isValidPropertyKey(prop.getKey()));
+      if (prop.getType().equals(PropertyType.PREFIX)) {
+        assertTrue(Property.isValidPropertyKey(prop.getKey() + "foo9"));
+      }
+    }
+
+    assertFalse(Property.isValidPropertyKey("abc.def"));
+  }
+
+  @Test
+  public void testIsValidTablePropertyKey() {
+    for (Property prop : Property.values()) {
+      if (prop.getKey().startsWith("table.") && !prop.getKey().equals("table."))
{
+        assertTrue(Property.isValidTablePropertyKey(prop.getKey()));
+
+        if (prop.getType().equals(PropertyType.PREFIX)) {
+          assertTrue(Property.isValidTablePropertyKey(prop.getKey() + "foo9"));
+        } else {
+          assertFalse(Property.isValidTablePropertyKey(prop.getKey() + "foo9"));
+        }
+      } else {
+        assertFalse(Property.isValidTablePropertyKey(prop.getKey()));
+      }
+
+    }
+
+    assertFalse(Property.isValidTablePropertyKey("abc.def"));
+  }
 }

-- 
To stop receiving notification emails like this one, please contact
kturner@apache.org.

Mime
View raw message