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:10 UTC
[accumulo] branch 1.7 updated (b52db47 -> 7b123b6)
This is an automated email from the ASF dual-hosted git repository.
kturner pushed a change to branch 1.7
in repository https://gitbox.apache.org/repos/asf/accumulo.git.
from b52db47 ACCUMULO-4777 Refine retry logging
new cf0b8aa Revert "ACCUMULO-4779 Speedup Property by precomputing and avoiding sync (#366)"
new 505d999 Revert "ACCUMULO-4778 Cache table name to id map (#364)"
new 51cc987 Revert "ACCUMULO-4779 Fix warning; remove unused field"
new 0b446a3 Revert "ACCUMULO-4779 made getting vfs config more efficient"
new 8d3bcda Revert "ACCUMULO-4779 Avoid locks in ZooCache when data in cache"
new 7b123b6 Revert "ACCUMULO-4779 fixed classpath context config performance issue (#358)"
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../accumulo/core/client/impl/ClientContext.java | 6 -
.../client/impl/MultiTableBatchWriterImpl.java | 79 ++++++++++-
.../apache/accumulo/core/client/impl/TableMap.java | 100 -------------
.../apache/accumulo/core/client/impl/Tables.java | 123 ++++++++--------
.../core/client/mock/MockConfiguration.java | 5 -
.../accumulo/core/conf/AccumuloConfiguration.java | 27 ----
.../accumulo/core/conf/ConfigurationCopy.java | 5 -
.../accumulo/core/conf/DefaultConfiguration.java | 5 -
.../org/apache/accumulo/core/conf/Property.java | 154 ++++++++-------------
.../accumulo/core/conf/SiteConfiguration.java | 9 --
.../apache/accumulo/core/conf/PropertyTest.java | 58 --------
.../apache/accumulo/fate/zookeeper/ZooCache.java | 97 ++++---------
.../accumulo/server/conf/ZooConfiguration.java | 17 +--
.../java/org/apache/accumulo/master/Master.java | 8 +-
.../org/apache/accumulo/tserver/TabletServer.java | 8 +-
.../main/java/org/apache/accumulo/shell/Shell.java | 18 +--
.../start/classloader/vfs/ContextManager.java | 39 ++++--
.../accumulo/test/MultiTableBatchWriterIT.java | 119 +++++++++++++++-
18 files changed, 387 insertions(+), 490 deletions(-)
delete mode 100644 core/src/main/java/org/apache/accumulo/core/client/impl/TableMap.java
--
To stop receiving notification emails like this one, please contact
kturner@apache.org.
[accumulo] 04/06: Revert "ACCUMULO-4779 made getting vfs config
more efficient"
Posted by kt...@apache.org.
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 0b446a31cb7425f153bd7278e739c216cf26e8a4
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Feb 2 12:13:44 2018 -0500
Revert "ACCUMULO-4779 made getting vfs config more efficient"
This reverts commit 7bc630cdce4a0aa02be4873e2873ff1036a94e14.
---
.../accumulo/core/client/impl/ClientContext.java | 6 -----
.../core/client/mock/MockConfiguration.java | 5 ----
.../accumulo/core/conf/AccumuloConfiguration.java | 27 ----------------------
.../accumulo/core/conf/ConfigurationCopy.java | 5 ----
.../accumulo/core/conf/DefaultConfiguration.java | 5 ----
.../accumulo/core/conf/SiteConfiguration.java | 9 --------
.../accumulo/server/conf/ZooConfiguration.java | 15 ++----------
.../java/org/apache/accumulo/master/Master.java | 4 ++--
.../org/apache/accumulo/tserver/TabletServer.java | 4 ++--
.../main/java/org/apache/accumulo/shell/Shell.java | 4 ++--
.../start/classloader/vfs/ContextManager.java | 11 ++++-----
11 files changed, 13 insertions(+), 82 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
index cb1ccb9..6828174 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
@@ -176,12 +176,6 @@ public class ClientContext {
final AccumuloConfiguration defaults = DefaultConfiguration.getInstance();
return new AccumuloConfiguration() {
-
- @Override
- protected String getArbitrarySystemPropertyImpl(String property) {
- return config.getString(property, null);
- }
-
@Override
public String get(Property property) {
final String key = property.getKey();
diff --git a/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java b/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
index f642770..410105b 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
@@ -36,11 +36,6 @@ class MockConfiguration extends AccumuloConfiguration {
}
@Override
- protected String getArbitrarySystemPropertyImpl(String property) {
- return map.get(property);
- }
-
- @Override
public String get(Property property) {
return map.get(property.getKey());
}
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
index 008418e..25d4c60 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
@@ -33,7 +33,6 @@ import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
@@ -107,32 +106,6 @@ public abstract class AccumuloConfiguration implements Iterable<Entry<String,Str
private static final Logger log = LoggerFactory.getLogger(AccumuloConfiguration.class);
- protected String getArbitrarySystemPropertyImpl(AccumuloConfiguration parent, String property) {
- return parent.getArbitrarySystemPropertyImpl(property);
- }
-
- /**
- * This method is not called with sensitive or per table properties.
- */
- protected String getArbitrarySystemPropertyImpl(String property) {
- throw new UnsupportedOperationException();
- }
-
- /**
- * This method was created because {@link #get(String)} is very slow. However this method does not properly handle everything that {@link #get(String)} does.
- * For example it does not properly handle table or sensitive properties.
- *
- * <p>
- * This method has a whitelist of prefixes it handles. To see the whitelist, check the implementation. When adding to the whitelist, ensure that all
- * configurations can properly handle.
- */
- public String getArbitrarySystemProperty(Property prefix, String property) {
- Preconditions.checkArgument(prefix == Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
-
- String key = prefix.getKey() + property;
- return getArbitrarySystemPropertyImpl(key);
- }
-
/**
* Gets a property value from this configuration.
*
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java
index df43557..28b188f 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java
@@ -64,11 +64,6 @@ public class ConfigurationCopy extends AccumuloConfiguration {
}
@Override
- protected String getArbitrarySystemPropertyImpl(String property) {
- return copy.get(property);
- }
-
- @Override
public void getProperties(Map<String,String> props, Predicate<String> filter) {
for (Entry<String,String> entry : copy.entrySet()) {
if (filter.apply(entry.getKey())) {
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 994d960..e1ff7e1 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
@@ -58,9 +58,4 @@ public class DefaultConfiguration extends AccumuloConfiguration {
if (filter.apply(entry.getKey()))
props.put(entry.getKey(), entry.getValue());
}
-
- @Override
- protected String getArbitrarySystemPropertyImpl(String property) {
- return null;
- }
}
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
index 32b2564..9f047e2 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
@@ -129,15 +129,6 @@ public class SiteConfiguration extends AccumuloConfiguration {
}
@Override
- protected String getArbitrarySystemPropertyImpl(String property) {
- String val = staticConfigs.get(property);
- if (val == null) {
- val = parent.getArbitrarySystemPropertyImpl(property);
- }
- return val;
- }
-
- @Override
public void getProperties(Map<String,String> props, Predicate<String> filter) {
parent.getProperties(props, filter);
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
index 2817ba0..f178ff1 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
@@ -40,13 +40,11 @@ public class ZooConfiguration extends AccumuloConfiguration {
private final ZooCache propCache;
private final AccumuloConfiguration parent;
private final Map<String,String> fixedProps = Collections.synchronizedMap(new HashMap<String,String>());
- private final String propPathPrefix;
protected ZooConfiguration(String instanceId, ZooCache propCache, AccumuloConfiguration parent) {
this.instanceId = instanceId;
this.propCache = propCache;
this.parent = parent;
- this.propPathPrefix = ZooUtil.getRoot(instanceId) + Constants.ZCONFIG;
}
@Override
@@ -81,15 +79,6 @@ public class ZooConfiguration extends AccumuloConfiguration {
}
@Override
- protected String getArbitrarySystemPropertyImpl(String property) {
- String val = getRaw(property);
- if (val == null) {
- val = getArbitrarySystemPropertyImpl(parent, property);
- }
- return val;
- }
-
- @Override
public String get(Property property) {
if (Property.isFixedZooPropertyKey(property)) {
if (fixedProps.containsKey(property.getKey())) {
@@ -108,7 +97,7 @@ public class ZooConfiguration extends AccumuloConfiguration {
}
private String getRaw(String key) {
- String zPath = propPathPrefix + "/" + key;
+ String zPath = ZooUtil.getRoot(instanceId) + Constants.ZCONFIG + "/" + key;
byte[] v = propCache.get(zPath);
String value = null;
if (v != null)
@@ -120,7 +109,7 @@ public class ZooConfiguration extends AccumuloConfiguration {
public void getProperties(Map<String,String> props, Predicate<String> filter) {
parent.getProperties(props, filter);
- List<String> children = propCache.getChildren(propPathPrefix);
+ List<String> children = propCache.getChildren(ZooUtil.getRoot(instanceId) + Constants.ZCONFIG);
if (children != null) {
for (String child : children) {
if (child != null && filter.apply(child)) {
diff --git a/server/master/src/main/java/org/apache/accumulo/master/Master.java b/server/master/src/main/java/org/apache/accumulo/master/Master.java
index 740e05f..984fb60 100644
--- a/server/master/src/main/java/org/apache/accumulo/master/Master.java
+++ b/server/master/src/main/java/org/apache/accumulo/master/Master.java
@@ -591,8 +591,8 @@ public class Master extends AccumuloServerContext implements LiveTServerSet.List
try {
AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
@Override
- public String getVfsContextClasspathProperty(String key) {
- return getConfiguration().getArbitrarySystemProperty(Property.VFS_CONTEXT_CLASSPATH_PROPERTY, key);
+ public String getProperty(String key) {
+ return getConfiguration().get(key);
}
});
} catch (IOException e) {
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index 7809e2e..5240db6 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -2816,8 +2816,8 @@ public class TabletServer extends AccumuloServerContext implements Runnable {
try {
AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
@Override
- public String getVfsContextClasspathProperty(String key) {
- return getConfiguration().getArbitrarySystemProperty(Property.VFS_CONTEXT_CLASSPATH_PROPERTY, key);
+ public String getProperty(String key) {
+ return getConfiguration().get(key);
}
});
} catch (IOException e) {
diff --git a/shell/src/main/java/org/apache/accumulo/shell/Shell.java b/shell/src/main/java/org/apache/accumulo/shell/Shell.java
index b1e473d..b5e5afb 100644
--- a/shell/src/main/java/org/apache/accumulo/shell/Shell.java
+++ b/shell/src/main/java/org/apache/accumulo/shell/Shell.java
@@ -550,8 +550,8 @@ public class Shell extends ShellOptions implements KeywordExecutable {
AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
@Override
- public String getVfsContextClasspathProperty(String key) {
- return systemConfig.get(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey() + key);
+ public String getProperty(String key) {
+ return systemConfig.get(key);
}
});
diff --git a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
index e155b3d..a3d51fc 100644
--- a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
+++ b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
@@ -99,21 +99,20 @@ public class ContextManager {
public static abstract class DefaultContextsConfig implements ContextsConfig {
- /**
- * Implementations should prepend {@link AccumuloVFSClassLoader#VFS_CONTEXT_CLASSPATH_PROPERTY} to the given key.
- */
- public abstract String getVfsContextClasspathProperty(String key);
+ public abstract String getProperty(String key);
@Override
public ContextConfig getContextConfig(String context) {
- String uris = getVfsContextClasspathProperty(context);
+ String key = AccumuloVFSClassLoader.VFS_CONTEXT_CLASSPATH_PROPERTY + context;
+
+ String uris = getProperty(key);
if (uris == null) {
return null;
}
- String delegate = getVfsContextClasspathProperty(context + ".delegation");
+ String delegate = getProperty(key + ".delegation");
boolean preDelegate = true;
--
To stop receiving notification emails like this one, please contact
kturner@apache.org.
[accumulo] 01/06: Revert "ACCUMULO-4779 Speedup Property by
precomputing and avoiding sync (#366)"
Posted by kt...@apache.org.
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.
[accumulo] 06/06: Revert "ACCUMULO-4779 fixed classpath context
config performance issue (#358)"
Posted by kt...@apache.org.
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 7b123b6ea77c5ba09f1875dad0f0f421096e4f2b
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Feb 2 12:15:44 2018 -0500
Revert "ACCUMULO-4779 fixed classpath context config performance issue (#358)"
This reverts commit 8c27bdf0e0168f854e9bb6effad25e6b6b0cb853.
---
.../java/org/apache/accumulo/master/Master.java | 8 ++---
.../org/apache/accumulo/tserver/TabletServer.java | 8 ++---
.../main/java/org/apache/accumulo/shell/Shell.java | 18 ++++++-----
.../start/classloader/vfs/ContextManager.java | 36 ++++++++++++++--------
4 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/server/master/src/main/java/org/apache/accumulo/master/Master.java b/server/master/src/main/java/org/apache/accumulo/master/Master.java
index 984fb60..6a43e93 100644
--- a/server/master/src/main/java/org/apache/accumulo/master/Master.java
+++ b/server/master/src/main/java/org/apache/accumulo/master/Master.java
@@ -589,12 +589,12 @@ public class Master extends AccumuloServerContext implements LiveTServerSet.List
this.tabletBalancer.init(serverConfig);
try {
- AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
+ AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig(new Iterable<Entry<String,String>>() {
@Override
- public String getProperty(String key) {
- return getConfiguration().get(key);
+ public Iterator<Entry<String,String>> iterator() {
+ return getConfiguration().iterator();
}
- });
+ }));
} catch (IOException e) {
throw new RuntimeException(e);
}
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index 5240db6..4db28b5 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -2814,12 +2814,12 @@ public class TabletServer extends AccumuloServerContext implements Runnable {
clientAddress = HostAndPort.fromParts(hostname, 0);
try {
- AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
+ AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig(new Iterable<Entry<String,String>>() {
@Override
- public String getProperty(String key) {
- return getConfiguration().get(key);
+ public Iterator<Entry<String,String>> iterator() {
+ return getConfiguration().iterator();
}
- });
+ }));
} catch (IOException e) {
throw new RuntimeException(e);
}
diff --git a/shell/src/main/java/org/apache/accumulo/shell/Shell.java b/shell/src/main/java/org/apache/accumulo/shell/Shell.java
index b5e5afb..36c843e 100644
--- a/shell/src/main/java/org/apache/accumulo/shell/Shell.java
+++ b/shell/src/main/java/org/apache/accumulo/shell/Shell.java
@@ -545,16 +545,18 @@ public class Shell extends ShellOptions implements KeywordExecutable {
shellState.getConnector().instanceOperations().getSystemConfiguration().get(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey() + classpath);
try {
-
- final Map<String,String> systemConfig = shellState.getConnector().instanceOperations().getSystemConfiguration();
-
- AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
+ AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig(new Iterable<Map.Entry<String,String>>() {
@Override
- public String getProperty(String key) {
- return systemConfig.get(key);
+ public Iterator<Entry<String,String>> iterator() {
+ try {
+ return shellState.getConnector().instanceOperations().getSystemConfiguration().entrySet().iterator();
+ } catch (AccumuloException e) {
+ throw new RuntimeException(e);
+ } catch (AccumuloSecurityException e) {
+ throw new RuntimeException(e);
+ }
}
-
- });
+ }));
} catch (IllegalStateException ise) {}
classloader = AccumuloVFSClassLoader.getContextManager().getClassLoader(classpath);
diff --git a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
index a3d51fc..7145b4a 100644
--- a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
+++ b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
@@ -18,7 +18,9 @@ package org.apache.accumulo.start.classloader.vfs;
import java.io.IOException;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import org.apache.commons.vfs2.FileSystemException;
@@ -97,30 +99,38 @@ public class ContextManager {
ContextConfig getContextConfig(String context);
}
- public static abstract class DefaultContextsConfig implements ContextsConfig {
+ public static class DefaultContextsConfig implements ContextsConfig {
- public abstract String getProperty(String key);
+ private Iterable<Entry<String,String>> config;
+
+ public DefaultContextsConfig(Iterable<Entry<String,String>> config) {
+ this.config = config;
+ }
@Override
public ContextConfig getContextConfig(String context) {
String key = AccumuloVFSClassLoader.VFS_CONTEXT_CLASSPATH_PROPERTY + context;
- String uris = getProperty(key);
-
- if (uris == null) {
- return null;
- }
-
- String delegate = getProperty(key + ".delegation");
-
+ String uris = null;
boolean preDelegate = true;
- if (delegate != null && delegate.trim().equalsIgnoreCase("post")) {
- preDelegate = false;
+ Iterator<Entry<String,String>> iter = config.iterator();
+ while (iter.hasNext()) {
+ Entry<String,String> entry = iter.next();
+ if (entry.getKey().equals(key)) {
+ uris = entry.getValue();
+ }
+
+ if (entry.getKey().equals(key + ".delegation") && entry.getValue().trim().equalsIgnoreCase("post")) {
+ preDelegate = false;
+ }
}
- return new ContextConfig(uris, preDelegate);
+ if (uris != null)
+ return new ContextConfig(uris, preDelegate);
+
+ return null;
}
}
--
To stop receiving notification emails like this one, please contact
kturner@apache.org.
[accumulo] 05/06: Revert "ACCUMULO-4779 Avoid locks in ZooCache
when data in cache"
Posted by kt...@apache.org.
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 8d3bcdaf345af2a57666b148bf1176561fa797e9
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Feb 2 12:13:59 2018 -0500
Revert "ACCUMULO-4779 Avoid locks in ZooCache when data in cache"
This reverts commit cf9e754b045e3fac452df282bcf4ec97974038a0.
---
.../apache/accumulo/fate/zookeeper/ZooCache.java | 97 ++++++----------------
1 file changed, 26 insertions(+), 71 deletions(-)
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
index 0c82d41..ba837f8 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
@@ -28,7 +28,6 @@ import java.util.ConcurrentModificationException;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.LockSupport;
import java.util.concurrent.locks.ReadWriteLock;
@@ -44,7 +43,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
/**
* A cache for values stored in ZooKeeper. Values are kept up to date as they change.
@@ -65,38 +63,6 @@ public class ZooCache {
private final ZooReader zReader;
- private static class ImmutableCacheCopies {
- final Map<String,byte[]> cache;
- final Map<String,Stat> statCache;
- final Map<String,List<String>> childrenCache;
-
- ImmutableCacheCopies() {
- cache = Collections.emptyMap();
- statCache = Collections.emptyMap();
- childrenCache = Collections.emptyMap();
- }
-
- ImmutableCacheCopies(Map<String,byte[]> cache, Map<String,Stat> statCache, Map<String,List<String>> childrenCache) {
- this.cache = Collections.unmodifiableMap(new HashMap<>(cache));
- this.statCache = Collections.unmodifiableMap(new HashMap<>(statCache));
- this.childrenCache = Collections.unmodifiableMap(new HashMap<>(childrenCache));
- }
-
- ImmutableCacheCopies(ImmutableCacheCopies prev, Map<String,List<String>> childrenCache) {
- this.cache = prev.cache;
- this.statCache = prev.statCache;
- this.childrenCache = Collections.unmodifiableMap(new HashMap<>(childrenCache));
- }
-
- ImmutableCacheCopies(Map<String,byte[]> cache, Map<String,Stat> statCache, ImmutableCacheCopies prev) {
- this.cache = Collections.unmodifiableMap(new HashMap<>(cache));
- this.statCache = Collections.unmodifiableMap(new HashMap<>(statCache));
- this.childrenCache = prev.childrenCache;
- }
- }
-
- private volatile ImmutableCacheCopies immutableCache = new ImmutableCacheCopies();
-
/**
* Returns a ZooKeeper session. Calls should be made within run of ZooRunnable after caches are checked. This will be performed at each retry of the run
* method. Calls to {@link #getZooKeeper()} should be made, ideally, after cache checks since other threads may have succeeded when updating the cache. Doing
@@ -257,17 +223,19 @@ public class ZooCache {
* path of node
* @return children list, or null if node has no children or does not exist
*/
- public List<String> getChildren(final String zPath) {
+ public synchronized List<String> getChildren(final String zPath) {
ZooRunnable<List<String>> zr = new ZooRunnable<List<String>>() {
@Override
public List<String> run() throws KeeperException, InterruptedException {
-
- // only read volatile once for consistency
- ImmutableCacheCopies lic = immutableCache;
- if (lic.childrenCache.containsKey(zPath)) {
- return lic.childrenCache.get(zPath);
+ try {
+ cacheReadLock.lock();
+ if (childrenCache.containsKey(zPath)) {
+ return childrenCache.get(zPath);
+ }
+ } finally {
+ cacheReadLock.unlock();
}
cacheWriteLock.lock();
@@ -279,11 +247,7 @@ public class ZooCache {
final ZooKeeper zooKeeper = getZooKeeper();
List<String> children = zooKeeper.getChildren(zPath, watcher);
- if (children != null) {
- children = ImmutableList.copyOf(children);
- }
childrenCache.put(zPath, children);
- immutableCache = new ImmutableCacheCopies(immutableCache, childrenCache);
return children;
} catch (KeeperException ke) {
if (ke.code() != Code.NONODE) {
@@ -297,7 +261,12 @@ public class ZooCache {
};
- return zr.retry();
+ List<String> children = zr.retry();
+
+ if (children == null) {
+ return null;
+ }
+ return Collections.unmodifiableList(children);
}
/**
@@ -326,16 +295,15 @@ public class ZooCache {
@Override
public byte[] run() throws KeeperException, InterruptedException {
Stat stat = null;
-
- // only read volatile once so following code works with a consistent snapshot
- ImmutableCacheCopies lic = immutableCache;
- byte[] val = lic.cache.get(zPath);
- if (val != null || lic.cache.containsKey(zPath)) {
- if (status != null) {
- stat = lic.statCache.get(zPath);
+ cacheReadLock.lock();
+ try {
+ if (cache.containsKey(zPath)) {
+ stat = statCache.get(zPath);
copyStats(status, stat);
+ return cache.get(zPath);
}
- return val;
+ } finally {
+ cacheReadLock.unlock();
}
/*
@@ -366,6 +334,7 @@ public class ZooCache {
}
put(zPath, data, stat);
copyStats(status, stat);
+ statCache.put(zPath, stat);
return data;
} finally {
cacheWriteLock.unlock();
@@ -408,8 +377,6 @@ public class ZooCache {
try {
cache.put(zPath, data);
statCache.put(zPath, stat);
-
- immutableCache = new ImmutableCacheCopies(cache, statCache, immutableCache);
} finally {
cacheWriteLock.unlock();
}
@@ -421,8 +388,6 @@ public class ZooCache {
cache.remove(zPath);
childrenCache.remove(zPath);
statCache.remove(zPath);
-
- immutableCache = new ImmutableCacheCopies(cache, statCache, childrenCache);
} finally {
cacheWriteLock.unlock();
}
@@ -431,14 +396,12 @@ public class ZooCache {
/**
* Clears this cache.
*/
- public void clear() {
+ public synchronized void clear() {
cacheWriteLock.lock();
try {
cache.clear();
childrenCache.clear();
statCache.clear();
-
- immutableCache = new ImmutableCacheCopies();
} finally {
cacheWriteLock.unlock();
}
@@ -452,14 +415,8 @@ public class ZooCache {
* @return true if data value is cached
*/
@VisibleForTesting
- boolean dataCached(String zPath) {
- cacheReadLock.lock();
- try {
- return immutableCache.cache.containsKey(zPath) && cache.containsKey(zPath);
- } finally {
- cacheReadLock.unlock();
- }
-
+ synchronized boolean dataCached(String zPath) {
+ return cache.containsKey(zPath);
}
/**
@@ -473,7 +430,7 @@ public class ZooCache {
boolean childrenCached(String zPath) {
cacheReadLock.lock();
try {
- return immutableCache.childrenCache.containsKey(zPath) && childrenCache.containsKey(zPath);
+ return childrenCache.containsKey(zPath);
} finally {
cacheReadLock.unlock();
}
@@ -505,8 +462,6 @@ public class ZooCache {
if (path.startsWith(zPath))
i.remove();
}
-
- immutableCache = new ImmutableCacheCopies(cache, statCache, childrenCache);
} finally {
cacheWriteLock.unlock();
}
--
To stop receiving notification emails like this one, please contact
kturner@apache.org.
[accumulo] 02/06: Revert "ACCUMULO-4778 Cache table name to id map
(#364)"
Posted by kt...@apache.org.
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 505d999b6944d8a91201257a4891a1ced6f64e25
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Feb 2 12:12:25 2018 -0500
Revert "ACCUMULO-4778 Cache table name to id map (#364)"
This reverts commit 5adeb4b7ed561a0bcea1a1def17835310831662f.
---
.../client/impl/MultiTableBatchWriterImpl.java | 79 ++++++++++++-
.../apache/accumulo/core/client/impl/TableMap.java | 100 -----------------
.../apache/accumulo/core/client/impl/Tables.java | 123 +++++++++++----------
.../accumulo/test/MultiTableBatchWriterIT.java | 119 +++++++++++++++++++-
4 files changed, 259 insertions(+), 162 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java b/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
index e7a6d73..f5e1fa0 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java
@@ -19,26 +19,37 @@ package org.apache.accumulo.core.client.impl;
import static com.google.common.base.Preconditions.checkArgument;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
import org.apache.accumulo.core.client.AccumuloException;
import org.apache.accumulo.core.client.AccumuloSecurityException;
import org.apache.accumulo.core.client.BatchWriter;
import org.apache.accumulo.core.client.BatchWriterConfig;
+import org.apache.accumulo.core.client.Instance;
import org.apache.accumulo.core.client.MultiTableBatchWriter;
import org.apache.accumulo.core.client.MutationsRejectedException;
import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.accumulo.core.client.TableOfflineException;
import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.master.state.tables.TableState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
import com.google.common.util.concurrent.UncheckedExecutionException;
public class MultiTableBatchWriterImpl implements MultiTableBatchWriter {
+ public static final long DEFAULT_CACHE_TIME = 200;
+ public static final TimeUnit DEFAULT_CACHE_TIME_UNIT = TimeUnit.MILLISECONDS;
private static final Logger log = LoggerFactory.getLogger(MultiTableBatchWriterImpl.class);
private AtomicBoolean closed;
+ private AtomicLong cacheLastState;
private class TableBatchWriter implements BatchWriter {
@@ -71,17 +82,49 @@ public class MultiTableBatchWriterImpl implements MultiTableBatchWriter {
}
+ /**
+ * CacheLoader which will look up the internal table ID for a given table name.
+ */
+ private class TableNameToIdLoader extends CacheLoader<String,String> {
+
+ @Override
+ public String load(String tableName) throws Exception {
+ Instance instance = context.getInstance();
+ String tableId = Tables.getNameToIdMap(instance).get(tableName);
+
+ if (tableId == null)
+ throw new TableNotFoundException(null, tableName, null);
+
+ if (Tables.getTableState(instance, tableId) == TableState.OFFLINE)
+ throw new TableOfflineException(instance, tableId);
+
+ return tableId;
+ }
+
+ }
+
private TabletServerBatchWriter bw;
private ConcurrentHashMap<String,BatchWriter> tableWriters;
private final ClientContext context;
+ private final LoadingCache<String,String> nameToIdCache;
public MultiTableBatchWriterImpl(ClientContext context, BatchWriterConfig config) {
+ this(context, config, DEFAULT_CACHE_TIME, DEFAULT_CACHE_TIME_UNIT);
+ }
+
+ public MultiTableBatchWriterImpl(ClientContext context, BatchWriterConfig config, long cacheTime, TimeUnit cacheTimeUnit) {
checkArgument(context != null, "context is null");
checkArgument(config != null, "config is null");
+ checkArgument(cacheTimeUnit != null, "cacheTimeUnit is null");
this.context = context;
this.bw = new TabletServerBatchWriter(context, config);
tableWriters = new ConcurrentHashMap<>();
this.closed = new AtomicBoolean(false);
+ this.cacheLastState = new AtomicLong(0);
+
+ // Potentially up to ~500k used to cache names to IDs with "segments" of (maybe) ~1000 entries
+ nameToIdCache = CacheBuilder.newBuilder().expireAfterWrite(cacheTime, cacheTimeUnit).concurrencyLevel(10).maximumSize(10000).initialCapacity(20)
+ .build(new TableNameToIdLoader());
}
@Override
@@ -118,7 +161,7 @@ public class MultiTableBatchWriterImpl implements MultiTableBatchWriter {
*/
private String getId(String tableName) throws TableNotFoundException {
try {
- return Tables.getTableId(context.inst, tableName);
+ return nameToIdCache.get(tableName);
} catch (UncheckedExecutionException e) {
Throwable cause = e.getCause();
@@ -133,6 +176,20 @@ public class MultiTableBatchWriterImpl implements MultiTableBatchWriter {
}
throw e;
+ } catch (ExecutionException e) {
+ Throwable cause = e.getCause();
+
+ log.error("Unexpected exception when fetching table id for " + tableName);
+
+ if (null == cause) {
+ throw new RuntimeException(e);
+ } else if (cause instanceof TableNotFoundException) {
+ throw (TableNotFoundException) cause;
+ } else if (cause instanceof TableOfflineException) {
+ throw (TableOfflineException) cause;
+ }
+
+ throw new RuntimeException(e);
}
}
@@ -140,6 +197,26 @@ public class MultiTableBatchWriterImpl implements MultiTableBatchWriter {
public BatchWriter getBatchWriter(String tableName) throws AccumuloException, AccumuloSecurityException, TableNotFoundException {
checkArgument(tableName != null, "tableName is null");
+ while (true) {
+ long cacheResetCount = Tables.getCacheResetCount();
+
+ // cacheResetCount could change after this point in time, but I think thats ok because just want to ensure this methods sees changes
+ // made before it was called.
+
+ long internalResetCount = cacheLastState.get();
+
+ if (cacheResetCount > internalResetCount) {
+ if (!cacheLastState.compareAndSet(internalResetCount, cacheResetCount)) {
+ continue; // concurrent operation, lets not possibly move cacheLastState backwards in the case where a thread pauses for along time
+ }
+
+ nameToIdCache.invalidateAll();
+ break;
+ }
+
+ break;
+ }
+
String tableId = getId(tableName);
BatchWriter tbw = tableWriters.get(tableId);
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/TableMap.java b/core/src/main/java/org/apache/accumulo/core/client/impl/TableMap.java
deleted file mode 100644
index 3f3d90c..0000000
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/TableMap.java
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * 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.client.impl;
-
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.apache.accumulo.core.client.impl.Tables.qualified;
-
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import org.apache.accumulo.core.Constants;
-import org.apache.accumulo.core.client.Instance;
-import org.apache.accumulo.core.client.NamespaceNotFoundException;
-import org.apache.accumulo.core.zookeeper.ZooUtil;
-import org.apache.accumulo.fate.zookeeper.ZooCache;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.collect.ImmutableMap;
-
-/**
- * Used for thread safe caching of immutable table ID maps. See ACCUMULO-4778.
- */
-public class TableMap {
- private static final Logger log = LoggerFactory.getLogger(TableMap.class);
-
- private final Map<String,String> tableNameToIdMap;
- private final Map<String,String> tableIdToNameMap;
-
- public TableMap(Instance instance, ZooCache zooCache) {
- List<String> tableIds = zooCache.getChildren(ZooUtil.getRoot(instance) + Constants.ZTABLES);
- Map<String,String> namespaceIdToNameMap = new HashMap<>();
- ImmutableMap.Builder<String,String> tableNameToIdBuilder = new ImmutableMap.Builder<>();
- ImmutableMap.Builder<String,String> tableIdToNameBuilder = new ImmutableMap.Builder<>();
- // use StringBuilder to construct zPath string efficiently across many tables
- StringBuilder zPathBuilder = new StringBuilder();
- zPathBuilder.append(ZooUtil.getRoot(instance)).append(Constants.ZTABLES).append("/");
- int prefixLength = zPathBuilder.length();
-
- for (String tableId : tableIds) {
- // reset StringBuilder to prefix length before appending ID and suffix
- zPathBuilder.setLength(prefixLength);
- zPathBuilder.append(tableId).append(Constants.ZTABLE_NAME);
- byte[] tableName = zooCache.get(zPathBuilder.toString());
- zPathBuilder.setLength(prefixLength);
- zPathBuilder.append(tableId).append(Constants.ZTABLE_NAMESPACE);
- byte[] nId = zooCache.get(zPathBuilder.toString());
-
- String namespaceName = Namespaces.DEFAULT_NAMESPACE;
- // create fully qualified table name
- if (nId == null) {
- namespaceName = null;
- } else {
- String namespaceId = new String(nId, UTF_8);
- if (!namespaceId.equals(Namespaces.DEFAULT_NAMESPACE_ID)) {
- try {
- namespaceName = namespaceIdToNameMap.get(namespaceId);
- if (namespaceName == null) {
- namespaceName = Namespaces.getNamespaceName(instance, namespaceId);
- namespaceIdToNameMap.put(namespaceId, namespaceName);
- }
- } catch (NamespaceNotFoundException e) {
- log.error("Table (" + tableId + ") contains reference to namespace (" + namespaceId + ") that doesn't exist", e);
- continue;
- }
- }
- }
- if (tableName != null && namespaceName != null) {
- String tableNameStr = qualified(new String(tableName, UTF_8), namespaceName);
- tableNameToIdBuilder.put(tableNameStr, tableId);
- tableIdToNameBuilder.put(tableId, tableNameStr);
- }
- }
- tableNameToIdMap = tableNameToIdBuilder.build();
- tableIdToNameMap = tableIdToNameBuilder.build();
- }
-
- public Map<String,String> getNameToIdMap() {
- return tableNameToIdMap;
- }
-
- public Map<String,String> getIdtoNameMap() {
- return tableIdToNameMap;
- }
-}
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java b/core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java
index a93347c..fcf838f 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java
@@ -20,11 +20,12 @@ import static com.google.common.base.Preconditions.checkArgument;
import static java.nio.charset.StandardCharsets.UTF_8;
import java.security.SecurityPermission;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.client.Instance;
@@ -36,49 +37,64 @@ import org.apache.accumulo.core.util.Pair;
import org.apache.accumulo.core.zookeeper.ZooUtil;
import org.apache.accumulo.fate.zookeeper.ZooCache;
import org.apache.accumulo.fate.zookeeper.ZooCacheFactory;
-import org.apache.zookeeper.WatchedEvent;
-import org.apache.zookeeper.Watcher;
-
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class Tables {
+ private static final Logger log = LoggerFactory.getLogger(Tables.class);
public static final String VALID_NAME_REGEX = "^(\\w+\\.)?(\\w+)$";
private static final SecurityPermission TABLES_PERMISSION = new SecurityPermission("tablesPermission");
- // Per instance cache will expire after 10 minutes in case we encounter an instance not used frequently
- private static Cache<String,TableMap> instanceToMapCache = CacheBuilder.newBuilder().expireAfterAccess(10, TimeUnit.MINUTES).build();
- private static Cache<String,ZooCache> instanceToZooCache = CacheBuilder.newBuilder().expireAfterAccess(10, TimeUnit.MINUTES).build();
+ private static final AtomicLong cacheResetCount = new AtomicLong(0);
- /**
- * Return the cached ZooCache for provided instance. ZooCache is initially created with a watcher that will clear the TableMap cache for that instance when
- * WatchedEvent occurs.
- */
- private static ZooCache getZooCache(final Instance instance) {
+ private static ZooCache getZooCache(Instance instance) {
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(TABLES_PERMISSION);
}
- final String zks = instance.getZooKeepers();
- final int timeOut = instance.getZooKeepersSessionTimeOut();
- final String uuid = instance.getInstanceID();
+ return new ZooCacheFactory().getZooCache(instance.getZooKeepers(), instance.getZooKeepersSessionTimeOut());
+ }
- try {
- return instanceToZooCache.get(uuid, new Callable<ZooCache>() {
- @Override
- public ZooCache call() {
- return new ZooCacheFactory().getZooCache(zks, timeOut, new Watcher() {
- @Override
- public void process(WatchedEvent watchedEvent) {
- instanceToMapCache.invalidate(uuid);
+ private static SortedMap<String,String> getMap(Instance instance, boolean nameAsKey) {
+ ZooCache zc = getZooCache(instance);
+
+ List<String> tableIds = zc.getChildren(ZooUtil.getRoot(instance) + Constants.ZTABLES);
+ TreeMap<String,String> tableMap = new TreeMap<>();
+ Map<String,String> namespaceIdToNameMap = new HashMap<>();
+
+ for (String tableId : tableIds) {
+ byte[] tableName = zc.get(ZooUtil.getRoot(instance) + Constants.ZTABLES + "/" + tableId + Constants.ZTABLE_NAME);
+ byte[] nId = zc.get(ZooUtil.getRoot(instance) + Constants.ZTABLES + "/" + tableId + Constants.ZTABLE_NAMESPACE);
+ String namespaceName = Namespaces.DEFAULT_NAMESPACE;
+ // create fully qualified table name
+ if (nId == null) {
+ namespaceName = null;
+ } else {
+ String namespaceId = new String(nId, UTF_8);
+ if (!namespaceId.equals(Namespaces.DEFAULT_NAMESPACE_ID)) {
+ try {
+ namespaceName = namespaceIdToNameMap.get(namespaceId);
+ if (namespaceName == null) {
+ namespaceName = Namespaces.getNamespaceName(instance, namespaceId);
+ namespaceIdToNameMap.put(namespaceId, namespaceName);
}
- });
+ } catch (NamespaceNotFoundException e) {
+ log.error("Table (" + tableId + ") contains reference to namespace (" + namespaceId + ") that doesn't exist", e);
+ continue;
+ }
}
- });
- } catch (ExecutionException e) {
- throw new RuntimeException(e);
+ }
+ if (tableName != null && namespaceName != null) {
+ String tableNameStr = qualified(new String(tableName, UTF_8), namespaceName);
+ if (nameAsKey)
+ tableMap.put(tableNameStr, tableId);
+ else
+ tableMap.put(tableId, tableNameStr);
+ }
}
+
+ return tableMap;
}
public static String getTableId(Instance instance, String tableName) throws TableNotFoundException {
@@ -113,31 +129,12 @@ public class Tables {
return tableName;
}
- public static Map<String,String> getNameToIdMap(Instance instance) {
- return getTableMap(instance).getNameToIdMap();
- }
-
- public static Map<String,String> getIdToNameMap(Instance instance) {
- return getTableMap(instance).getIdtoNameMap();
+ public static SortedMap<String,String> getNameToIdMap(Instance instance) {
+ return getMap(instance, true);
}
- /**
- * Get the TableMap from the cache. A new one will be populated when needed. Cache is cleared manually by calling {@link #clearCache(Instance)} or
- * automatically cleared by ZooCache watcher created in {@link #getZooCache(Instance)}. See ACCUMULO-4778.
- */
- private static TableMap getTableMap(final Instance instance) {
- TableMap map;
- try {
- map = instanceToMapCache.get(instance.getInstanceID(), new Callable<TableMap>() {
- @Override
- public TableMap call() {
- return new TableMap(instance, getZooCache(instance));
- }
- });
- } catch (ExecutionException e) {
- throw new RuntimeException(e);
- }
- return map;
+ public static SortedMap<String,String> getIdToNameMap(Instance instance) {
+ return getMap(instance, false);
}
public static boolean exists(Instance instance, String tableId) {
@@ -147,9 +144,9 @@ public class Tables {
}
public static void clearCache(Instance instance) {
+ cacheResetCount.incrementAndGet();
getZooCache(instance).clear(ZooUtil.getRoot(instance) + Constants.ZTABLES);
getZooCache(instance).clear(ZooUtil.getRoot(instance) + Constants.ZNAMESPACES);
- instanceToMapCache.invalidate(instance.getInstanceID());
}
/**
@@ -161,9 +158,17 @@ public class Tables {
* A zookeeper path
*/
public static void clearCacheByPath(Instance instance, final String zooPath) {
- String thePath = zooPath.startsWith("/") ? zooPath : "/" + zooPath;
+
+ String thePath;
+
+ if (zooPath.startsWith("/")) {
+ thePath = zooPath;
+ } else {
+ thePath = "/" + zooPath;
+ }
+
getZooCache(instance).clear(ZooUtil.getRoot(instance) + thePath);
- instanceToMapCache.invalidate(instance.getInstanceID());
+
}
public static String getPrintableTableNameFromId(Map<String,String> tidToNameMap, String tableId) {
@@ -224,6 +229,10 @@ public class Tables {
}
+ public static long getCacheResetCount() {
+ return cacheResetCount.get();
+ }
+
public static String qualified(String tableName) {
return qualified(tableName, Namespaces.DEFAULT_NAMESPACE);
}
diff --git a/test/src/test/java/org/apache/accumulo/test/MultiTableBatchWriterIT.java b/test/src/test/java/org/apache/accumulo/test/MultiTableBatchWriterIT.java
index fa5d8bb..f9720f0 100644
--- a/test/src/test/java/org/apache/accumulo/test/MultiTableBatchWriterIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/MultiTableBatchWriterIT.java
@@ -20,6 +20,7 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.concurrent.TimeUnit;
import org.apache.accumulo.core.client.AccumuloException;
import org.apache.accumulo.core.client.AccumuloSecurityException;
@@ -30,6 +31,7 @@ import org.apache.accumulo.core.client.MultiTableBatchWriter;
import org.apache.accumulo.core.client.MutationsRejectedException;
import org.apache.accumulo.core.client.Scanner;
import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.client.TableOfflineException;
import org.apache.accumulo.core.client.admin.TableOperations;
import org.apache.accumulo.core.client.impl.ClientContext;
import org.apache.accumulo.core.client.impl.Credentials;
@@ -59,12 +61,12 @@ public class MultiTableBatchWriterIT extends AccumuloClusterIT {
@Before
public void setUpArgs() throws AccumuloException, AccumuloSecurityException {
connector = getConnector();
- mtbw = getMultiTableBatchWriter();
+ mtbw = getMultiTableBatchWriter(60);
}
- public MultiTableBatchWriter getMultiTableBatchWriter() {
+ public MultiTableBatchWriter getMultiTableBatchWriter(long cacheTimeoutInSeconds) {
ClientContext context = new ClientContext(connector.getInstance(), new Credentials(getAdminPrincipal(), getAdminToken()), getCluster().getClientConfig());
- return new MultiTableBatchWriterImpl(context, new BatchWriterConfig());
+ return new MultiTableBatchWriterImpl(context, new BatchWriterConfig(), cacheTimeoutInSeconds, TimeUnit.SECONDS);
}
@Test
@@ -263,7 +265,7 @@ public class MultiTableBatchWriterIT extends AccumuloClusterIT {
@Test
public void testTableRenameNewWritersNoCaching() throws Exception {
- mtbw = getMultiTableBatchWriter();
+ mtbw = getMultiTableBatchWriter(0);
try {
final String[] names = getUniqueNames(4);
@@ -404,4 +406,113 @@ public class MultiTableBatchWriterIT extends AccumuloClusterIT {
Assert.assertTrue("Expected mutations to be rejected.", mutationsRejected);
}
+
+ @Test
+ public void testOfflineTableWithCache() throws Exception {
+ boolean mutationsRejected = false;
+
+ try {
+ final String[] names = getUniqueNames(2);
+ final String table1 = names[0], table2 = names[1];
+
+ TableOperations tops = connector.tableOperations();
+ tops.create(table1);
+ tops.create(table2);
+
+ BatchWriter bw1 = mtbw.getBatchWriter(table1), bw2 = mtbw.getBatchWriter(table2);
+
+ Mutation m1 = new Mutation("foo");
+ m1.put("col1", "", "val1");
+ m1.put("col2", "", "val2");
+
+ bw1.addMutation(m1);
+ bw2.addMutation(m1);
+
+ tops.offline(table1);
+
+ try {
+ bw1 = mtbw.getBatchWriter(table1);
+ } catch (TableOfflineException e) {
+ // pass
+ mutationsRejected = true;
+ }
+
+ tops.offline(table2);
+
+ try {
+ bw2 = mtbw.getBatchWriter(table2);
+ } catch (TableOfflineException e) {
+ // pass
+ mutationsRejected = true;
+ }
+ } finally {
+ if (null != mtbw) {
+ try {
+ // Mutations might have flushed before the table offline occurred
+ mtbw.close();
+ } catch (MutationsRejectedException e) {
+ // Pass
+ mutationsRejected = true;
+ }
+ }
+ }
+
+ Assert.assertTrue("Expected mutations to be rejected.", mutationsRejected);
+ }
+
+ @Test
+ public void testOfflineTableWithoutCache() throws Exception {
+ mtbw = getMultiTableBatchWriter(0);
+ boolean mutationsRejected = false;
+
+ try {
+ final String[] names = getUniqueNames(2);
+ final String table1 = names[0], table2 = names[1];
+
+ TableOperations tops = connector.tableOperations();
+ tops.create(table1);
+ tops.create(table2);
+
+ BatchWriter bw1 = mtbw.getBatchWriter(table1), bw2 = mtbw.getBatchWriter(table2);
+
+ Mutation m1 = new Mutation("foo");
+ m1.put("col1", "", "val1");
+ m1.put("col2", "", "val2");
+
+ bw1.addMutation(m1);
+ bw2.addMutation(m1);
+
+ // Mutations might or might not flush before tables goes offline
+ tops.offline(table1);
+ tops.offline(table2);
+
+ try {
+ bw1 = mtbw.getBatchWriter(table1);
+ Assert.fail(table1 + " should be offline");
+ } catch (TableOfflineException e) {
+ // pass
+ mutationsRejected = true;
+ }
+
+ try {
+ bw2 = mtbw.getBatchWriter(table2);
+ Assert.fail(table1 + " should be offline");
+ } catch (TableOfflineException e) {
+ // pass
+ mutationsRejected = true;
+ }
+ } finally {
+ if (null != mtbw) {
+ try {
+ // Mutations might have flushed before the table offline occurred
+ mtbw.close();
+ } catch (MutationsRejectedException e) {
+ // Pass
+ mutationsRejected = true;
+ }
+ }
+ }
+
+ Assert.assertTrue("Expected mutations to be rejected.", mutationsRejected);
+ }
}
--
To stop receiving notification emails like this one, please contact
kturner@apache.org.
[accumulo] 03/06: Revert "ACCUMULO-4779 Fix warning;
remove unused field"
Posted by kt...@apache.org.
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 51cc9874a34a7890fa645b034a0038d041ed2216
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Feb 2 12:13:17 2018 -0500
Revert "ACCUMULO-4779 Fix warning; remove unused field"
This reverts commit d8d2afa11037e41504d8f9347c1f6d36002e4a6a.
---
.../src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java | 2 ++
1 file changed, 2 insertions(+)
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
index cb7bc98..2817ba0 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
@@ -36,12 +36,14 @@ import com.google.common.base.Predicate;
public class ZooConfiguration extends AccumuloConfiguration {
private static final Logger log = LoggerFactory.getLogger(ZooConfiguration.class);
+ private final String instanceId;
private final ZooCache propCache;
private final AccumuloConfiguration parent;
private final Map<String,String> fixedProps = Collections.synchronizedMap(new HashMap<String,String>());
private final String propPathPrefix;
protected ZooConfiguration(String instanceId, ZooCache propCache, AccumuloConfiguration parent) {
+ this.instanceId = instanceId;
this.propCache = propCache;
this.parent = parent;
this.propPathPrefix = ZooUtil.getRoot(instanceId) + Constants.ZCONFIG;
--
To stop receiving notification emails like this one, please contact
kturner@apache.org.