[GitHub] keith-turner commented on a change in pull request #840: fixes #819 Handle bad locality group config.

[GitBox <gi...@apache.org>]

keith-turner commented on a change in pull request #840: fixes #819 Handle bad locality group config.
URL: https://github.com/apache/accumulo/pull/840#discussion_r243443980
 
 

 ##########
 File path: core/src/main/java/org/apache/accumulo/core/client/impl/InstanceOperationsImpl.java
 ##########
 @@ -84,6 +89,22 @@ public void execute(MasterClientService.Client client) throws Exception {
         client.removeSystemProperty(Tracer.traceInfo(), context.rpcCreds(), property);
       }
     });
+
+    checkLocalityGroups(property);
+  }
+
+  void checkLocalityGroups(String propChanged) throws AccumuloSecurityException, AccumuloException {
+    if (LocalityGroupUtil.isLocalityGroupProperty(propChanged)) {
+      try {
+        LocalityGroupUtil.checkLocalityGroups(getSystemConfiguration().entrySet());
+      } catch (LocalityGroupConfigurationError | RuntimeException e) {
+        LoggerFactory.getLogger(this.getClass()).warn("Changing '" + propChanged
+            + "' resulted in bad locality group config. This may be a transient situation since "
+            + "the config spreads over multiple properties. Setting properties in a different "
+            + "order may help. Even though this warning was displayed, the property was updated. "
+            + "Please check your config to ensure consistency.", e);
 
 Review comment:
   > Why update the property if it is going to result in a bad locality group?
   
   It's because locality group config is spread across multiple properties.  So there could be existing user code like the following.
   
   ```
    setProp(k1,v1); //makes LG group config incomplete/bad
    setProp(k2,v2); //makes LG group config incomplete/bad
    setProp(k3,v3); //makes LG config complete/good
   ```
   
   So the transient bad state is very short lived and the user may have never noticed any problems from it.  I do not want to break existing code that is working for 1.9.3.  However it would be good to bring this to the users attention.
   

----------------------------------------------------------------
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