You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by kt...@apache.org on 2018/02/02 18:25:11 UTC
[accumulo] 01/06: Revert "ACCUMULO-4779 Speedup Property by
precomputing and avoiding sync (#366)"
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
commit cf0b8aa4994e10242478c4d9412d924d7e6de91c
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Feb 2 12:11:53 2018 -0500
Revert "ACCUMULO-4779 Speedup Property by precomputing and avoiding sync (#366)"
This reverts commit 1fe3ba12a943e590b89b2979e661e7dc447d0774.
---
.../org/apache/accumulo/core/conf/Property.java | 154 ++++++++-------------
.../apache/accumulo/core/conf/PropertyTest.java | 58 --------
2 files changed, 56 insertions(+), 156 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 1733124..5a32a83 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,8 +39,6 @@ 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
@@ -591,16 +589,7 @@ public enum Property {
;
- 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 String key, defaultValue, description;
private PropertyType type;
private static final Logger log = LoggerFactory.getLogger(Property.class);
@@ -640,11 +629,6 @@ 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();
@@ -659,9 +643,7 @@ public enum Property {
}
if (this.type == PropertyType.ABSOLUTEPATH && !(v.trim().equals("")))
v = new File(v).getAbsolutePath();
-
- computedDefaultValue = v;
- defaultValueComputed = true;
+ return v;
}
/**
@@ -683,8 +665,7 @@ public enum Property {
}
private boolean isInterpolated() {
- Preconditions.checkState(annotationsComputed, "precomputeAnnotations() must be called before calling this method");
- return isInterpolated;
+ return hasAnnotation(Interpolated.class) || hasPrefixWithAnnotation(getKey(), Interpolated.class);
}
/**
@@ -693,8 +674,7 @@ public enum Property {
* @return true if this property is experimental
*/
public boolean isExperimental() {
- Preconditions.checkState(annotationsComputed, "precomputeAnnotations() must be called before calling this method");
- return isExperimental;
+ return hasAnnotation(Experimental.class) || hasPrefixWithAnnotation(getKey(), Experimental.class);
}
/**
@@ -703,26 +683,21 @@ public enum Property {
* @return true if this property is deprecated
*/
public boolean isDeprecated() {
- Preconditions.checkState(annotationsComputed, "precomputeAnnotations() must be called before calling this method");
- return isDeprecated;
+ return hasAnnotation(Deprecated.class) || hasPrefixWithAnnotation(getKey(), Deprecated.class);
}
+ private volatile Boolean isSensitive = null;
+
/**
* Checks if this property is sensitive.
*
* @return true if this property is sensitive
*/
public boolean isSensitive() {
- 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;
+ if (isSensitive == null) {
+ isSensitive = hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class);
+ }
+ return isSensitive.booleanValue();
}
/**
@@ -734,20 +709,7 @@ public enum Property {
* @return true if property is sensitive
*/
public static boolean isSensitive(String key) {
- 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;
+ return hasPrefixWithAnnotation(key, Sensitive.class);
}
private <T extends Annotation> boolean hasAnnotation(Class<T> annotationType) {
@@ -765,21 +727,24 @@ public enum Property {
}
private static <T extends Annotation> boolean hasPrefixWithAnnotation(String key, Class<T> annotationType) {
- for (String prefix : validPrefixes) {
- if (key.startsWith(prefix)) {
- if (propertiesByKey.get(prefix).hasAnnotation(annotationType)) {
- return true;
- }
- }
+ // 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;
}
-
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) {
@@ -798,7 +763,20 @@ public enum Property {
* property key
* @return true if key is valid (recognized, or has a recognized prefix)
*/
- public static boolean isValidPropertyKey(String key) {
+ 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());
+ }
+ }
+ }
+
return validProperties.contains(key) || isKeyValidlyPrefixed(key);
}
@@ -811,12 +789,20 @@ public enum Property {
* property key
* @return true if key is valid for a table property (recognized, or has a recognized prefix)
*/
- 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())));
+ 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());
}
/**
@@ -852,7 +838,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(REPLICATION_PREFIX.getKey());
+ || key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey()) || key.startsWith(REPLICATION_PREFIX.getKey());
}
/**
@@ -863,7 +849,10 @@ public enum Property {
* @return property, or null if not found
*/
public static Property getPropertyByKey(String key) {
- return propertiesByKey.get(key);
+ for (Property prop : Property.values())
+ if (prop.getKey().equals(key))
+ return prop;
+ return null;
}
/**
@@ -963,35 +952,4 @@ 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 d25c8c2..6848ee4 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,7 +19,6 @@ 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;
@@ -153,61 +152,4 @@ 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.