You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2018/01/26 01:03:33 UTC
[GitHub] keith-turner closed pull request #367: ACCUMULO-4779 made getting vfs config more efficient
keith-turner closed pull request #367: ACCUMULO-4779 made getting vfs config more efficient
URL: https://github.com/apache/accumulo/pull/367
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 6828174b61..cb1ccb93f7 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,6 +176,12 @@ public static AccumuloConfiguration convertClientConfig(final Configuration conf
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 410105b5ac..f642770d2f 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
@@ -35,6 +35,11 @@ public void put(String k, String v) {
map.put(k, v);
}
+ @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 25d4c60064..008418e185 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,6 +33,7 @@
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;
@@ -106,6 +107,32 @@ public boolean apply(String key) {
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 28b188fb1a..df4355760f 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
@@ -63,6 +63,11 @@ public String get(Property property) {
return copy.get(property.getKey());
}
+ @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()) {
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 e1ff7e16bb..994d960e02 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,4 +58,9 @@ public void getProperties(Map<String,String> props, Predicate<String> filter) {
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 9f047e2ba4..32b2564a0f 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
@@ -128,6 +128,15 @@ public String get(Property property) {
return value;
}
+ @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 f178ff1ef1..2817ba0c4d 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,11 +40,13 @@
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
@@ -78,6 +80,15 @@ private String _get(Property property) {
return value;
}
+ @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)) {
@@ -97,7 +108,7 @@ public String get(Property property) {
}
private String getRaw(String key) {
- String zPath = ZooUtil.getRoot(instanceId) + Constants.ZCONFIG + "/" + key;
+ String zPath = propPathPrefix + "/" + key;
byte[] v = propCache.get(zPath);
String value = null;
if (v != null)
@@ -109,7 +120,7 @@ private String getRaw(String key) {
public void getProperties(Map<String,String> props, Predicate<String> filter) {
parent.getProperties(props, filter);
- List<String> children = propCache.getChildren(ZooUtil.getRoot(instanceId) + Constants.ZCONFIG);
+ List<String> children = propCache.getChildren(propPathPrefix);
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 984fb60494..740e05f844 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 Master(ServerConfigurationFactory config, VolumeManager fs, String hostna
try {
AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
@Override
- public String getProperty(String key) {
- return getConfiguration().get(key);
+ public String getVfsContextClasspathProperty(String key) {
+ return getConfiguration().getArbitrarySystemProperty(Property.VFS_CONTEXT_CLASSPATH_PROPERTY, 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 852c61139a..eecac468aa 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
@@ -2813,8 +2813,8 @@ public void config(String hostname) {
try {
AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
@Override
- public String getProperty(String key) {
- return getConfiguration().get(key);
+ public String getVfsContextClasspathProperty(String key) {
+ return getConfiguration().getArbitrarySystemProperty(Property.VFS_CONTEXT_CLASSPATH_PROPERTY, 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 b5e5afb6ce..b1e473dfac 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 ClassLoader getClassLoader(final CommandLine cl, final Shell shellState)
AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig() {
@Override
- public String getProperty(String key) {
- return systemConfig.get(key);
+ public String getVfsContextClasspathProperty(String key) {
+ return systemConfig.get(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey() + 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 a3d51fcff5..e155b3df8e 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,20 +99,21 @@ public int hashCode() {
public static abstract class DefaultContextsConfig implements ContextsConfig {
- public abstract String getProperty(String key);
+ /**
+ * Implementations should prepend {@link AccumuloVFSClassLoader#VFS_CONTEXT_CLASSPATH_PROPERTY} to the given key.
+ */
+ public abstract String getVfsContextClasspathProperty(String key);
@Override
public ContextConfig getContextConfig(String context) {
- String key = AccumuloVFSClassLoader.VFS_CONTEXT_CLASSPATH_PROPERTY + context;
-
- String uris = getProperty(key);
+ String uris = getVfsContextClasspathProperty(context);
if (uris == null) {
return null;
}
- String delegate = getProperty(key + ".delegation");
+ String delegate = getVfsContextClasspathProperty(context + ".delegation");
boolean preDelegate = true;
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services