You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by mw...@apache.org on 2018/11/02 16:40:58 UTC
[accumulo] branch master updated: Avoid ZK watch when checking if
property is set (#740)
This is an automated email from the ASF dual-hosted git repository.
mwalch pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/master by this push:
new f220675 Avoid ZK watch when checking if property is set (#740)
f220675 is described below
commit f22067517691c88dac390e1aa7fc9fe3f398c4f0
Author: Mike Walch <mw...@apache.org>
AuthorDate: Fri Nov 2 12:40:53 2018 -0400
Avoid ZK watch when checking if property is set (#740)
* Updated AccumuloConfig.isPropertySet() method optionally cache and
watch for changes in the property
---
.../accumulo/core/conf/AccumuloConfiguration.java | 6 ++--
.../accumulo/core/conf/DefaultConfiguration.java | 2 +-
.../accumulo/core/conf/SiteConfiguration.java | 4 +--
.../core/conf/AccumuloConfigurationTest.java | 2 +-
.../org/apache/accumulo/server/ServerUtil.java | 2 +-
.../accumulo/server/conf/ZooConfiguration.java | 34 ++++++++++++++++++----
.../server/conf/ZooConfigurationFactory.java | 2 +-
.../server/conf/ZooConfigurationFactoryTest.java | 1 +
8 files changed, 38 insertions(+), 15 deletions(-)
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 8212f2c..64a05a9 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
@@ -386,7 +386,7 @@ public abstract class AccumuloConfiguration implements Iterable<Entry<String,Str
}
}
- public boolean isPropertySet(Property prop) {
+ public boolean isPropertySet(Property prop, boolean cacheAndWatch) {
throw new UnsupportedOperationException();
}
@@ -409,14 +409,14 @@ public abstract class AccumuloConfiguration implements Iterable<Entry<String,Str
return null;
}
- if (!isPropertySet(prop) && isPropertySet(deprecatedProp)) {
+ if (!isPropertySet(prop, true) && isPropertySet(deprecatedProp, true)) {
if (!depPropWarned) {
depPropWarned = true;
log.warn("Property {} is deprecated, use {} instead.", deprecatedProp.getKey(),
prop.getKey());
}
return Integer.valueOf(get(deprecatedProp));
- } else if (isPropertySet(prop) && isPropertySet(deprecatedProp) && !depPropWarned) {
+ } else if (isPropertySet(prop, true) && isPropertySet(deprecatedProp, true) && !depPropWarned) {
depPropWarned = true;
log.warn("Deprecated property {} ignored because {} is set", deprecatedProp.getKey(),
prop.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 3e82187..8d3ad4e 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
@@ -54,7 +54,7 @@ public class DefaultConfiguration extends AccumuloConfiguration {
}
@Override
- public boolean isPropertySet(Property prop) {
+ public boolean isPropertySet(Property prop, boolean cacheAndWatch) {
return false;
}
}
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 7a7e095..35f907b 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
@@ -221,7 +221,7 @@ public class SiteConfiguration extends AccumuloConfiguration {
}
@Override
- public boolean isPropertySet(Property prop) {
+ public boolean isPropertySet(Property prop, boolean cacheAndWatch) {
if (prop.isSensitive()) {
String hadoopVal = getSensitiveFromHadoop(prop);
if (hadoopVal != null) {
@@ -229,7 +229,7 @@ public class SiteConfiguration extends AccumuloConfiguration {
}
}
return overrides.containsKey(prop.getKey()) || staticConfigs.containsKey(prop.getKey())
- || getConfiguration().containsKey(prop.getKey()) || parent.isPropertySet(prop);
+ || getConfiguration().containsKey(prop.getKey()) || parent.isPropertySet(prop, cacheAndWatch);
}
@Override
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java b/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
index 491d0cd..c027c2e 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
@@ -151,7 +151,7 @@ public class AccumuloConfigurationTest {
}
@Override
- public boolean isPropertySet(Property prop) {
+ public boolean isPropertySet(Property prop, boolean cacheAndWatch) {
return props.containsKey(prop.getKey());
}
diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java b/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java
index 7466185..c9719fb 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/ServerUtil.java
@@ -126,7 +126,7 @@ public class ServerUtil {
String key = entry.getKey();
log.info("{} = {}", key, (Property.isSensitive(key) ? "<hidden>" : entry.getValue()));
Property prop = Property.getPropertyByKey(key);
- if (prop != null && conf.isPropertySet(prop)) {
+ if (prop != null && conf.isPropertySet(prop, false)) {
if (prop.isDeprecated()) {
Property replacedBy = prop.replacedBy();
if (replacedBy != null) {
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 9547d04..aeb4bca 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
@@ -28,22 +28,27 @@ import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.conf.AccumuloConfiguration;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.fate.zookeeper.ZooCache;
-import org.apache.accumulo.fate.zookeeper.ZooUtil;
+import org.apache.accumulo.fate.zookeeper.ZooReader;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class ZooConfiguration extends AccumuloConfiguration {
+
private static final Logger log = LoggerFactory.getLogger(ZooConfiguration.class);
+ private final ServerContext context;
private final ZooCache propCache;
private final AccumuloConfiguration parent;
private final Map<String,String> fixedProps = Collections.synchronizedMap(new HashMap<>());
private final String propPathPrefix;
- protected ZooConfiguration(String instanceId, ZooCache propCache, AccumuloConfiguration parent) {
+ protected ZooConfiguration(ServerContext context, ZooCache propCache, AccumuloConfiguration parent) {
+ this.context = context;
this.propCache = propCache;
this.parent = parent;
- this.propPathPrefix = ZooUtil.getRoot(instanceId) + Constants.ZCONFIG;
+ this.propPathPrefix = context.getZooKeeperRoot() + Constants.ZCONFIG;
}
@Override
@@ -98,9 +103,26 @@ public class ZooConfiguration extends AccumuloConfiguration {
}
@Override
- public boolean isPropertySet(Property prop) {
- return fixedProps.containsKey(prop.getKey()) || getRaw(prop.getKey()) != null
- || parent.isPropertySet(prop);
+ public boolean isPropertySet(Property prop, boolean cacheAndWatch) {
+ if (fixedProps.containsKey(prop.getKey())) {
+ return true;
+ }
+ if (cacheAndWatch) {
+ if (getRaw(prop.getKey()) != null) {
+ return true;
+ }
+ } else {
+ ZooReader zr = context.getZooReaderWriter();
+ String zPath = propPathPrefix + "/" + prop.getKey();
+ try {
+ if (zr.exists(zPath)) {
+ return true;
+ }
+ } catch (KeeperException|InterruptedException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+ return parent.isPropertySet(prop, cacheAndWatch);
}
private String getRaw(String key) {
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java
index 9449fe8..715b7c7 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java
@@ -61,7 +61,7 @@ class ZooConfigurationFactory {
};
propCache = zcf.getZooCache(context.getZooKeepers(), context.getZooKeepersSessionTimeOut(),
watcher);
- config = new ZooConfiguration(context.getInstanceID(), propCache, parent);
+ config = new ZooConfiguration(context, propCache, parent);
instances.put(context.getInstanceID(), config);
}
}
diff --git a/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java b/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java
index 9b23e3d..72e3d1b 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java
@@ -52,6 +52,7 @@ public class ZooConfigurationFactoryTest {
@Test
public void testGetInstance() {
+ expect(context.getZooKeeperRoot()).andReturn("zkroot").anyTimes();
expect(context.getInstanceID()).andReturn("iid").anyTimes();
expect(context.getZooKeepers()).andReturn("localhost").anyTimes();
expect(context.getZooKeepersSessionTimeOut()).andReturn(120000).anyTimes();