accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From els...@apache.org
Subject accumulo git commit: ACCUMULO-3604 patch to throw IllegalArgumentException for invalid property
Date Mon, 03 Aug 2015 19:10:44 GMT
Repository: accumulo
Updated Branches:
  refs/heads/master a9e6d7348 -> 5060e82dc


ACCUMULO-3604 patch to throw IllegalArgumentException for invalid property

Signed-off-by: Josh Elser <elserj@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/5060e82d
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/5060e82d
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/5060e82d

Branch: refs/heads/master
Commit: 5060e82dcbbff74a4aade59faa435a294ac6926e
Parents: a9e6d73
Author: Jeffrey S. Schwartz <jeff@Jeffreys-MacBook-Pro.local>
Authored: Fri Jul 31 13:11:25 2015 -0400
Committer: Josh Elser <elserj@apache.org>
Committed: Mon Aug 3 15:09:53 2015 -0400

----------------------------------------------------------------------
 .../core/client/admin/InstanceOperations.java   |  4 ++-
 .../client/impl/InstanceOperationsImpl.java     |  2 +-
 .../accumulo/server/util/SystemPropUtil.java    | 28 ++++++++++++----
 .../master/MasterClientServiceHandler.java      |  3 ++
 .../accumulo/test/ZooKeeperPropertiesIT.java    | 34 ++++++++++++++++++++
 5 files changed, 63 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/5060e82d/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
b/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
index 54194c1..79b430c 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
@@ -29,7 +29,9 @@ public interface InstanceOperations {
 
   /**
    * Sets an system property in zookeeper. Tablet servers will pull this setting and override
the equivalent setting in accumulo-site.xml. Changes can be seen
-   * using {@link #getSystemConfiguration()}
+   * using {@link #getSystemConfiguration()}.
+   * <p>
+   * Only some properties can be changed by this method, an IllegalArgumentException will
be thrown if a read-only property is set.
    *
    * @param property
    *          the name of a per-table property

http://git-wip-us.apache.org/repos/asf/accumulo/blob/5060e82d/core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java
b/core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java
index 6383967..32a7ec0 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java
@@ -63,7 +63,7 @@ public class InstanceOperationsImpl implements InstanceOperations {
   }
 
   @Override
-  public void setProperty(final String property, final String value) throws AccumuloException,
AccumuloSecurityException {
+  public void setProperty(final String property, final String value) throws AccumuloException,
AccumuloSecurityException, IllegalArgumentException {
     checkArgument(property != null, "property is null");
     checkArgument(value != null, "value is null");
     MasterClient.execute(context, new ClientExec<MasterClientService.Client>() {

http://git-wip-us.apache.org/repos/asf/accumulo/blob/5060e82d/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
index 49a6971..4880a56 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
@@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.conf.PropertyType;
 import org.apache.accumulo.core.zookeeper.ZooUtil;
 import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeExistsPolicy;
 import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeMissingPolicy;
@@ -33,17 +34,32 @@ public class SystemPropUtil {
   private static final Logger log = LoggerFactory.getLogger(SystemPropUtil.class);
 
   public static boolean setSystemProperty(String property, String value) throws KeeperException,
InterruptedException {
-    Property p = Property.getPropertyByKey(property);
-    if ((p != null && !p.getType().isValidFormat(value)) || !Property.isValidZooPropertyKey(property))
{
-      log.warn("Ignoring property {} it is null, an invalid format, or not capable of being
changed in zookeeper", property);
-      return false;
+    if (!Property.isValidZooPropertyKey(property)) {
+      IllegalArgumentException iae = new IllegalArgumentException("Zookeeper property is
not mutable: " + property);
+      log.debug("Attempted to set zookeeper property.  It is not mutable", iae);
+      throw iae;
+    }
+
+    // Find the property taking prefix into account
+    Property foundProp = null;
+    for (Property prop : Property.values()) {
+      if (PropertyType.PREFIX == prop.getType() && property.startsWith(prop.getKey())
|| prop.getKey().equals(property)) {
+        foundProp = prop;
+        break;
+      }
+    }
+
+    if ((foundProp == null || !foundProp.getType().isValidFormat(value))) {
+      IllegalArgumentException iae = new IllegalArgumentException("Ignoring property " +
property + " it is either null or in an invalid format");
+      log.debug("Attempted to set zookeeper property.  Value is either null or invalid",
iae);
+      throw iae;
     }
 
     // create the zk node for this property and set it's data to the specified value
     String zPath = ZooUtil.getRoot(HdfsZooInstance.getInstance()) + Constants.ZCONFIG + "/"
+ property;
-    ZooReaderWriter.getInstance().putPersistentData(zPath, value.getBytes(UTF_8), NodeExistsPolicy.OVERWRITE);
+    boolean result = ZooReaderWriter.getInstance().putPersistentData(zPath, value.getBytes(UTF_8),
NodeExistsPolicy.OVERWRITE);
 
-    return true;
+    return result;
   }
 
   public static void removeSystemProperty(String property) throws InterruptedException, KeeperException
{

http://git-wip-us.apache.org/repos/asf/accumulo/blob/5060e82d/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
----------------------------------------------------------------------
diff --git a/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
b/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
index 7d8f406..821dad9 100644
--- a/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
+++ b/server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
@@ -378,6 +378,9 @@ public class MasterClientServiceHandler extends FateServiceHandler implements
Ma
     try {
       SystemPropUtil.setSystemProperty(property, value);
       updatePlugins(property);
+    } catch (IllegalArgumentException iae) {
+      // throw the exception here so it is not caught and converted to a generic TException
+      throw iae;
     } catch (Exception e) {
       Master.log.error("Problem setting config property in zookeeper", e);
       throw new TException(e.getMessage());

http://git-wip-us.apache.org/repos/asf/accumulo/blob/5060e82d/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java
----------------------------------------------------------------------
diff --git a/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java b/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java
new file mode 100644
index 0000000..d1e4882
--- /dev/null
+++ b/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java
@@ -0,0 +1,34 @@
+/*
+ * 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.test;
+
+import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.Connector;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.harness.AccumuloClusterHarness;
+import org.junit.Test;
+
+public class ZooKeeperPropertiesIT extends AccumuloClusterHarness {
+
+  @Test(expected = AccumuloException.class)
+  public void testNoFiles() throws Exception {
+    Connector conn = getConnector();
+    // Should throw an error as this property can't be changed in ZooKeeper
+    conn.instanceOperations().setProperty(Property.GENERAL_RPC_TIMEOUT.getKey(), "60s");
+  }
+
+}


Mime
View raw message