[jira] [Commented] (ACCUMULO-3604) connector.instanceOperations().setSystemProperty(String,String) returns silently for non-changeable ZK Property's

["Josh Elser (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ACCUMULO-3604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14651152#comment-14651152 ] 

Josh Elser commented on ACCUMULO-3604:
--------------------------------------

Ah, I see. The root of the issue is probably because {{Property.getPropertyByKey}} doesn't return the {{Property}} when it's a prefix. However, changing that method seems to have some unintended consequences to it as well (other places in the code then start to fail).

Here's the change I made that made the test pass:

{code}
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 3d50e6f..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;
@@ -39,8 +40,16 @@ public class SystemPropUtil {
       throw iae;
     }

-    Property p = Property.getPropertyByKey(property);
-    if ((p == null || !p.getType().isValidFormat(value))) {
+    // 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;
{code}

What do you think?

> connector.instanceOperations().setSystemProperty(String,String) returns silently for non-changeable ZK Property's
> -----------------------------------------------------------------------------------------------------------------
>
>                 Key: ACCUMULO-3604
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-3604
>             Project: Accumulo
>          Issue Type: Bug
>          Components: master
>    Affects Versions: 1.5.0, 1.5.1, 1.5.2, 1.6.0, 1.6.1, 1.6.2
>            Reporter: Josh Elser
>            Assignee: Jeffrey S Schwartz
>              Labels: newbie, summit2015
>             Fix For: 1.8.0
>
>         Attachments: ACCUMULO-3604.patch, ACCUMULO-3604.v2.patch
>
>
> Only a subset of the configuration {{Property}}'s in Accumulo are modifiable via ZooKeeper (defined by {{Property.isValidZooProperty}}).
> {{connector.instanceOperations().setProperty(String,String)}} updates the provided property name with the given value in ZooKeeper. The thing that is never mentioned is that only a subset of the properties in Accumulo are allowed to be overriden in ZooKeeper. Furthermore, the user receives no indication that their call failed.
> The Javadoc on {{setSystemProperty(String,String)}} should be updated to inform the users that only some properties can be changed by this method, and some information should be returned back to the user to let them know that their call did not succeed (likely an Exception).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)