From 01a251b50a131c5efc2de17ea42508e547035777 Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Sat, 24 Jul 2010 05:08:21 +0000 Subject: [PATCH] HBASE-2874 Unnecessary double-synchronization in ZooKeeperWrapper git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@978801 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 2 + .../hbase/zookeeper/ZooKeeperWrapper.java | 94 +++++++++---------- 2 files changed, 49 insertions(+), 47 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 5f4b7aca3b6..7d3e9cd76e0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -799,6 +799,8 @@ Release 0.21.0 - Unreleased HBASE-2873 Minor clean up in basescanner; fix a log and make deletes of region processing run in order HBASE-2830 NotServingRegionException shouldn't log a stack trace + HBASE-2874 Unnecessary double-synchronization in ZooKeeperWrapper + (Benoît Sigoure via Stack) NEW FEATURES HBASE-1961 HBase EC2 scripts diff --git a/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java index d19c881fae8..3827fa5d5a3 100644 --- a/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java +++ b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java @@ -58,39 +58,39 @@ import org.apache.zookeeper.data.Stat; * This class provides methods to: * - read/write/delete the root region location in ZooKeeper. * - set/check out of safe mode flag. - * + * * ------------------------------------------ * The following STATIC ZNodes are created: * ------------------------------------------ - * - parentZNode : All the HBase directories are hosted under this parent + * - parentZNode : All the HBase directories are hosted under this parent * node, default = "/hbase" - * - rsZNode : This is the directory where the RS's create ephemeral - * nodes. The master watches these nodes, and their expiry + * - rsZNode : This is the directory where the RS's create ephemeral + * nodes. The master watches these nodes, and their expiry * indicates RS death. The default location is "/hbase/rs" - * + * * ------------------------------------------ * The following DYNAMIC ZNodes are created: * ------------------------------------------ * - rootRegionZNode : Specifies the RS hosting root. - * - masterElectionZNode : ZNode used for election of the primary master when - * there are secondaries. All the masters race to write - * their addresses into this location, the one that + * - masterElectionZNode : ZNode used for election of the primary master when + * there are secondaries. All the masters race to write + * their addresses into this location, the one that * succeeds is the primary. Others block. - * - clusterStateZNode : Determines if the cluster is running. Its default - * location is "/hbase/shutdown". It always has a value - * of "up". If present with the valus, cluster is up - * and running. If deleted, the cluster is shutting + * - clusterStateZNode : Determines if the cluster is running. Its default + * location is "/hbase/shutdown". It always has a value + * of "up". If present with the valus, cluster is up + * and running. If deleted, the cluster is shutting * down. - * - rgnsInTransitZNode : All the nodes under this node are names of regions - * in transition. The first byte of the data for each - * of these nodes is the event type. This is used to + * - rgnsInTransitZNode : All the nodes under this node are names of regions + * in transition. The first byte of the data for each + * of these nodes is the event type. This is used to * deserialize the rest of the data. */ public class ZooKeeperWrapper implements Watcher { protected static final Log LOG = LogFactory.getLog(ZooKeeperWrapper.class); // instances of the watcher - private static Map INSTANCES = + private static Map INSTANCES = new HashMap(); // lock for ensuring a singleton per instance type private static Lock createLock = new ReentrantLock(); @@ -112,13 +112,13 @@ public class ZooKeeperWrapper implements Watcher { * Specifies the RS hosting root */ private final String rootRegionZNode; - /* - * This is the directory where the RS's create ephemeral nodes. The master - * watches these nodes, and their expiry indicates RS death. + /* + * This is the directory where the RS's create ephemeral nodes. The master + * watches these nodes, and their expiry indicates RS death. */ private final String rsZNode; /* - * ZNode used for election of the primary master when there are secondaries. + * ZNode used for election of the primary master when there are secondaries. */ private final String masterElectionZNode; /* @@ -127,14 +127,14 @@ public class ZooKeeperWrapper implements Watcher { public final String clusterStateZNode; /* * Regions that are in transition - */ + */ private final String rgnsInTransitZNode; /* * List of ZNodes in the unassgined region that are already being watched */ private Set unassignedZNodesWatched = new HashSet(); - private List listeners = Collections.synchronizedList(new ArrayList()); + private List listeners = new ArrayList(); // return the singleton given the name of the instance public static ZooKeeperWrapper getInstance(Configuration conf, String name) { @@ -185,7 +185,7 @@ public class ZooKeeperWrapper implements Watcher { } sessionTimeout = conf.getInt("zookeeper.session.timeout", 60 * 1000); reconnectToZk(); - + parentZNode = conf.get(HConstants.ZOOKEEPER_ZNODE_PARENT, HConstants.DEFAULT_ZOOKEEPER_ZNODE_PARENT); String rootServerZNodeName = conf.get("zookeeper.znode.rootserver", "root-region-server"); @@ -200,7 +200,7 @@ public class ZooKeeperWrapper implements Watcher { masterElectionZNode = getZNode(parentZNode, masterAddressZNodeName); clusterStateZNode = getZNode(parentZNode, stateZNodeName); } - + public void reconnectToZk() throws IOException { try { LOG.info("Reconnecting to zookeeper"); @@ -216,7 +216,7 @@ public class ZooKeeperWrapper implements Watcher { } catch (InterruptedException e) { LOG.error("<" + instanceName + ">" + "Error closing ZK connection: " + e); throw new IOException(e); - } + } } public synchronized void registerListener(Watcher watcher) { @@ -748,11 +748,11 @@ public class ZooKeeperWrapper implements Watcher { LOG.warn("<" + instanceName + ">" + "Failed to delete " + rsZNode + " znodes in ZooKeeper: " + e); } } - + /** * @return the number of region server znodes in the RS directory */ - public int getRSDirectoryCount() { + public int getRSDirectoryCount() { Stat stat = null; try { stat = zooKeeper.exists(rsZNode, false); @@ -1072,19 +1072,19 @@ public class ZooKeeperWrapper implements Watcher { throw new IOException(e); } } - + /** - * Given a region name and some data, this method creates a new the region - * znode data under the UNASSGINED znode with the data passed in. This method + * Given a region name and some data, this method creates a new the region + * znode data under the UNASSGINED znode with the data passed in. This method * will not update data for existing znodes. - * + * * @param regionName - encoded name of the region * @param data - new serialized data to update the region znode */ private void createUnassignedRegion(String regionName, byte[] data) { String znode = getZNode(getRegionInTransitionZNode(), regionName); if(LOG.isDebugEnabled()) { - // check if this node already exists - + // check if this node already exists - // - it should not exist // - if it does, it should be in the CLOSED state if(exists(znode, true)) { @@ -1096,7 +1096,7 @@ public class ZooKeeperWrapper implements Watcher { LOG.error("Error reading data for " + znode); } if(oldData == null) { - LOG.debug("While creating UNASSIGNED region " + regionName + " exists with no data" ); + LOG.debug("While creating UNASSIGNED region " + regionName + " exists with no data" ); } else { LOG.debug("While creating UNASSIGNED region " + regionName + " exists, state = " + (HBaseEventType.fromByte(oldData[0]))); @@ -1104,7 +1104,7 @@ public class ZooKeeperWrapper implements Watcher { } else { if(data == null) { - LOG.debug("Creating UNASSIGNED region " + regionName + " with no data" ); + LOG.debug("Creating UNASSIGNED region " + regionName + " with no data" ); } else { LOG.debug("Creating UNASSIGNED region " + regionName + " in state = " + (HBaseEventType.fromByte(data[0]))); @@ -1118,10 +1118,10 @@ public class ZooKeeperWrapper implements Watcher { } /** - * Given a region name and some data, this method updates the region znode - * data under the UNASSGINED znode with the latest data. This method will + * Given a region name and some data, this method updates the region znode + * data under the UNASSGINED znode with the latest data. This method will * update the znode data only if it already exists. - * + * * @param regionName - encoded name of the region * @param data - new serialized data to update the region znode */ @@ -1132,7 +1132,7 @@ public class ZooKeeperWrapper implements Watcher { LOG.error("Cannot update " + znode + " - node does not exist" ); return; } - + Stat stat = new Stat(); byte[] oldData = null; try { @@ -1174,10 +1174,10 @@ public class ZooKeeperWrapper implements Watcher { } /** - * This method will create a new region in transition entry in ZK with the - * speficied data if none exists. If one already exists, it will update the + * This method will create a new region in transition entry in ZK with the + * speficied data if none exists. If one already exists, it will update the * data with whatever is passed in. - * + * * @param regionName - encoded name of the region * @param data - serialized data for the region znode */ @@ -1228,8 +1228,8 @@ public class ZooKeeperWrapper implements Watcher { } /** - * Atomically adds a watch and reads data from the unwatched znodes in the - * UNASSGINED region. This works because the master is the only person + * Atomically adds a watch and reads data from the unwatched znodes in the + * UNASSGINED region. This works because the master is the only person * deleting nodes. * @param znode * @return @@ -1258,22 +1258,22 @@ public class ZooKeeperWrapper implements Watcher { } return newNodes; } - + public static class ZNodePathAndData { private String zNodePath; private byte[] data; - + public ZNodePathAndData(String zNodePath, byte[] data) { this.zNodePath = zNodePath; this.data = data; } - + public String getzNodePath() { return zNodePath; } public byte[] getData() { return data; } - + } }