mikewalch closed pull request #740: Avoid ZK watch when checking if property is
set
URL: https://github.com/apache/accumulo/pull/740
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/conf/AccumuloConfiguration.java
b/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
index 8212f2c4eb..64a05a98e7 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 int getCurrentMaxThreads() {
}
}
- public boolean isPropertySet(Property prop) {
+ public boolean isPropertySet(Property prop, boolean cacheAndWatch) {
throw new UnsupportedOperationException();
}
@@ -409,14 +409,14 @@ Integer getDeprecatedScanThreads(String name) {
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 3e82187500..8d3ad4eaf3 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 void getProperties(Map<String,String> props,
Predicate<String> filter) {
}
@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 7a7e0955ba..35f907b1ff 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 @@ private String getSensitiveFromHadoop(Property property) {
}
@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 boolean isPropertySet(Property prop) {
}
}
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 491d0cda86..c027c2e45f 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 void set(String p, String v) {
}
@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 7466185070..c9719fbf70 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 static void init(ServerContext context, String
application) {
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 9547d046b4..aeb4bca1b7 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.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 String get(Property property) {
}
@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 9449fe8446..715b7c7e53 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 @@ public void process(WatchedEvent arg0) {}
};
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 9b23e3dac8..72e3d1b1e7 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 void setUp() {
@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();
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
