ZOOKEEPER-302. Quote values in JMX objectnames. (tom and pat via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@741729 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -112,6 +112,8 @@ and runping via mahadev)
   ZOOKEEPER-293. zoo_set needs to be abi compatible (3.1 changed the
 signature), fix this by adding zoo_set2 (pat via mahadev)
 
+  ZOOKEEPER-302. Quote values in JMX objectnames. (tom and pat via mahadev)
+
 IMPROVEMENTS:
    
   ZOOKEEPER-64. Log system env information when initializing server and

src/java/main/org/apache/zookeeper/jmx/MBeanRegistry.java

@@ -22,7 +22,9 @@ import java.lang.management.ManagementFactory;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+import javax.management.JMException;
 import javax.management.MBeanServer;
+import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
 
 import org.apache.log4j.Logger;
@@ -54,7 +56,9 @@ public class MBeanRegistry {
      * @param parent if not null, the new bean will be registered as a child
      * node of this parent.
      */
-    public void register(ZKMBeanInfo bean, ZKMBeanInfo parent) {
+    public void register(ZKMBeanInfo bean, ZKMBeanInfo parent)
+        throws JMException
+    {
         assert bean != null;
         String path = null;
         if (parent != null) {
@@ -70,9 +74,9 @@ public class MBeanRegistry {
         ObjectName oname = makeObjectName(path, bean);
         try {
             mbs.registerMBean(bean, oname);
-        } catch (Exception e) {
-            LOG.error("Failed to register MBean " + bean.getName());
-            e.printStackTrace();
+        } catch (JMException e) {
+            LOG.warn("Failed to register MBean " + bean.getName());
+            throw e;
         }
     }
 
@@ -81,16 +85,16 @@ public class MBeanRegistry {
      * @param path
      * @param bean
      */
-    private void unregister(String path,ZKMBeanInfo bean){
+    private void unregister(String path,ZKMBeanInfo bean) throws JMException {
         if(path==null)
             return;
         if (!bean.isHidden()) {
             MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
             try {
                 mbs.unregisterMBean(makeObjectName(path, bean));
-            } catch (Exception e) {
-                LOG.error("Failed to unregister MBean " + bean.getName());
-                e.printStackTrace();
+            } catch (JMException e) {
+                LOG.warn("Failed to unregister MBean " + bean.getName());
+                throw e;
             }
         }        
     }
@@ -99,20 +103,28 @@ public class MBeanRegistry {
      * Unregister MBean.
      * @param bean
      */
-    public void unregister(ZKMBeanInfo bean){
+    public void unregister(ZKMBeanInfo bean) {
         if(bean==null)
             return;
         String path=mapBean2Path.get(bean);
-        unregister(path,bean);
+        try {
+            unregister(path,bean);
+        } catch (JMException e) {
+            LOG.warn("Error during unregister", e);
+        }
         mapBean2Path.remove(bean);
         mapName2Bean.remove(bean.getName());
     }
     /**
      * Unregister all currently registered MBeans
      */
-    public void unregisterAll(){
-        for(Map.Entry<ZKMBeanInfo,String> e: mapBean2Path.entrySet()){
-            unregister(e.getValue(),e.getKey());
+    public void unregisterAll() {
+        for(Map.Entry<ZKMBeanInfo,String> e: mapBean2Path.entrySet()) {
+            try {
+                unregister(e.getValue(), e.getKey());
+            } catch (JMException e1) {
+                LOG.warn("Error during unregister", e1);
+            }
         }
         mapBean2Path.clear();
         mapName2Bean.clear();
@@ -123,7 +135,7 @@ public class MBeanRegistry {
      * @param name path elements
      * @return absolute path
      */
-    public String makeFullPath(String prefix, String... name){
+    public String makeFullPath(String prefix, String... name) {
         StringBuilder sb=new StringBuilder(prefix == null ? "/" : (prefix.equals("/")?prefix:prefix+"/"));
         boolean first=true;
         for (String s : name) {
@@ -159,9 +171,11 @@ public class MBeanRegistry {
      * Builds an MBean path and creates an ObjectName instance using the path. 
      * @param path MBean path
      * @param bean the MBean instance
-     * @return ObjectName to be registered with the paltform MBean server
+     * @return ObjectName to be registered with the platform MBean server
      */
-    protected ObjectName makeObjectName(String path, ZKMBeanInfo bean) {
+    protected ObjectName makeObjectName(String path, ZKMBeanInfo bean)
+        throws MalformedObjectNameException
+    {
         if(path==null)
             return null;
         StringBuilder beanName = new StringBuilder(CommonNames.DOMAIN + ":");
@@ -171,10 +185,10 @@ public class MBeanRegistry {
         beanName.deleteCharAt(beanName.length()-1);
         try {
             return new ObjectName(beanName.toString());
-        } catch (Exception e) {
-            e.printStackTrace();
-            assert false;
+        } catch (MalformedObjectNameException e) {
+            LOG.warn("Invalid name \"" + beanName.toString() + "\" for class "
+                    + bean.getClass().toString());
+            throw e;
         }
-        return null;
     }
 }

src/java/main/org/apache/zookeeper/server/ConnectionBean.java

@@ -18,9 +18,14 @@
 
 package org.apache.zookeeper.server;
 
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.util.Arrays;
 import java.util.Date;
 
+import javax.management.ObjectName;
+
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.jmx.MBeanRegistry;
 import org.apache.zookeeper.jmx.ZKMBeanInfo;
@@ -44,15 +49,20 @@ public class ConnectionBean implements ConnectionMXBean, ZKMBeanInfo {
         return "0x" + Long.toHexString(connection.getSessionId());
     }
 
-    
     public String getSourceIP() {
-        return connection.getRemoteAddress().getAddress().getHostAddress()+
-            ":"+connection.getRemoteAddress().getPort();
+        InetSocketAddress sockAddr = connection.getRemoteAddress();
+        return sockAddr.getAddress().getHostAddress()
+            + ":" + sockAddr.getPort();
     }
-    
+
     public String getName() {
-        String ip=connection.getRemoteAddress().getAddress().getHostAddress();
-        return MBeanRegistry.getInstance().makeFullPath("Connections", ip,getSessionId());
+        InetAddress addr = connection.getRemoteAddress().getAddress();
+        String ip = addr.getHostAddress();
+        if (addr instanceof Inet6Address) {
+            ip = ObjectName.quote(ip);
+        }
+        return MBeanRegistry.getInstance().makeFullPath("Connections", ip,
+                getSessionId());
     }
     
     public boolean isHidden() {
@@ -61,7 +71,8 @@ public class ConnectionBean implements ConnectionMXBean, ZKMBeanInfo {
     
     public String[] getEphemeralNodes() {
         if(zk.dataTree!=null){
-            String[] res=zk.dataTree.getEphemerals(connection.getSessionId()).toArray(new String[0]);
+            String[] res=zk.dataTree.getEphemerals(connection.getSessionId())
+                .toArray(new String[0]);
             Arrays.sort(res);
             return res;
         }
@@ -84,10 +95,11 @@ public class ConnectionBean implements ConnectionMXBean, ZKMBeanInfo {
     public void terminateConnection() {
         connection.close();
     }
-    
+
     @Override
     public String toString() {
-        return "ConnectionBean{ClientIP="+getSourceIP()+",SessionId=0x"+getSessionId()+"}";
+        return "ConnectionBean{ClientIP=" + ObjectName.quote(getSourceIP())
+            + ",SessionId=0x" + getSessionId() + "}";
     }
     
     public long getOutstandingRequests() {

src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java

@@ -861,6 +861,7 @@ public class NIOServerCnxn implements Watcher, ServerCnxn {
             MBeanRegistry.getInstance().register(jmxConnectionBean, zk.jmxServerBean);
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxConnectionBean = null;
         }
 
         try {

src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java

@@ -334,10 +334,17 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider {
         try {
             jmxServerBean = new ZooKeeperServerBean(this);
             MBeanRegistry.getInstance().register(jmxServerBean, null);
-            jmxDataTreeBean = new DataTreeBean(dataTree);
-            MBeanRegistry.getInstance().register(jmxDataTreeBean, jmxServerBean);
+            
+            try {
+                jmxDataTreeBean = new DataTreeBean(dataTree);
+                MBeanRegistry.getInstance().register(jmxDataTreeBean, jmxServerBean);
+            } catch (Exception e) {
+                LOG.warn("Failed to register with JMX", e);
+                jmxDataTreeBean = null;
+            }
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxServerBean = null;
         }
     }
     
@@ -397,6 +404,10 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider {
             if (jmxDataTreeBean != null) {
                 MBeanRegistry.getInstance().unregister(jmxDataTreeBean);
             }
+        } catch (Exception e) {
+            LOG.warn("Failed to unregister with JMX", e);
+        }
+        try {
             if (jmxServerBean != null) {
                 MBeanRegistry.getInstance().unregister(jmxServerBean);
             }

src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java

@@ -24,8 +24,6 @@ import java.util.Date;
 
 import org.apache.zookeeper.Version;
 import org.apache.zookeeper.jmx.ZKMBeanInfo;
-import org.apache.zookeeper.server.ServerStats;
-import org.apache.zookeeper.server.ZooKeeperServer;
 
 /**
  * This class implements the ZooKeeper server MBean interface.
@@ -45,7 +43,7 @@ public class ZooKeeperServerBean implements ZooKeeperServerMXBean, ZKMBeanInfo {
     public String getClientPort() {
         try {
             return InetAddress.getLocalHost().getHostAddress() + ":"
-                    + zks.getClientPort();
+                + zks.getClientPort();
         } catch (UnknownHostException e) {
             return "localhost:" + zks.getClientPort();
         }

src/java/main/org/apache/zookeeper/server/quorum/FollowerZooKeeperServer.java

@@ -187,6 +187,7 @@ public class FollowerZooKeeperServer extends ZooKeeperServer {
             MBeanRegistry.getInstance().register(jmxDataTreeBean, jmxServerBean);
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxDataTreeBean = null;
         }
     }
 
@@ -194,14 +195,21 @@ public class FollowerZooKeeperServer extends ZooKeeperServer {
             LocalPeerBean localPeerBean)
     {
         // register with JMX
-        try {
-            MBeanRegistry.getInstance().unregister(self.jmxLeaderElectionBean);
+        if (self.jmxLeaderElectionBean != null) {
+            try {
+                MBeanRegistry.getInstance().unregister(self.jmxLeaderElectionBean);
+            } catch (Exception e) {
+                LOG.warn("Failed to register with JMX", e);
+            }
             self.jmxLeaderElectionBean = null;
+        }
 
+        try {
             jmxServerBean = followerBean;
             MBeanRegistry.getInstance().register(followerBean, localPeerBean);
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxServerBean = null;
         }
     }
 

src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java

@@ -96,6 +96,7 @@ public class LeaderZooKeeperServer extends ZooKeeperServer {
             MBeanRegistry.getInstance().register(jmxDataTreeBean, jmxServerBean);
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxDataTreeBean = null;
         }
     }
 
@@ -103,14 +104,21 @@ public class LeaderZooKeeperServer extends ZooKeeperServer {
             LocalPeerBean localPeerBean)
     {
         // register with JMX
-        try {
-            MBeanRegistry.getInstance().unregister(self.jmxLeaderElectionBean);
+        if (self.jmxLeaderElectionBean != null) {
+            try {
+                MBeanRegistry.getInstance().unregister(self.jmxLeaderElectionBean);
+            } catch (Exception e) {
+                LOG.warn("Failed to register with JMX", e);
+            }
             self.jmxLeaderElectionBean = null;
+        }
 
+        try {
             jmxServerBean = leaderBean;
             MBeanRegistry.getInstance().register(leaderBean, localPeerBean);
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxServerBean = null;
         }
     }
 

src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java

@@ -372,6 +372,7 @@ public class QuorumPeer extends Thread implements QuorumStats.Provider {
             MBeanRegistry.getInstance().register(jmxLeaderElectionBean, jmxLocalPeerBean);        
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxLeaderElectionBean = null;
         }
 
         if(electionAlg==null)
@@ -407,13 +408,24 @@ public class QuorumPeer extends Thread implements QuorumStats.Provider {
                 ZKMBeanInfo p;
                 if (getId() == s.id) {
                     p = jmxLocalPeerBean = new LocalPeerBean(this);
+                    try {
+                        MBeanRegistry.getInstance().register(p, jmxQuorumBean);
+                    } catch (Exception e) {
+                        LOG.warn("Failed to register with JMX", e);
+                        jmxLocalPeerBean = null;
+                    }
                 } else {
                     p = new RemotePeerBean(s);
+                    try {
+                        MBeanRegistry.getInstance().register(p, jmxQuorumBean);
+                    } catch (Exception e) {
+                        LOG.warn("Failed to register with JMX", e);
+                    }
                 }
-                MBeanRegistry.getInstance().register(p, jmxQuorumBean);
             }
         } catch (Exception e) {
             LOG.warn("Failed to register with JMX", e);
+            jmxQuorumBean = null;
         }
 
         try {