From 74ae25d843c7ffc52992fccb092f81ad4979e589 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Sat, 13 Jul 2019 22:20:30 +0530 Subject: [PATCH] HBASE-22638 ZooKeeper Utility enhancements Signed-off-by: Jan Hentschel --- .../hbase/zookeeper/MiniZooKeeperCluster.java | 33 ++++----- .../hbase/zookeeper/RecoverableZooKeeper.java | 6 +- .../hadoop/hbase/zookeeper/ZKAclReset.java | 28 ++++---- .../hadoop/hbase/zookeeper/ZKClusterId.java | 4 +- .../hbase/zookeeper/ZKLeaderManager.java | 6 +- .../apache/hadoop/hbase/zookeeper/ZKUtil.java | 72 ++++++++++--------- 6 files changed, 78 insertions(+), 71 deletions(-) diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MiniZooKeeperCluster.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MiniZooKeeperCluster.java index 02e8b87a4c7..740e4b07b45 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MiniZooKeeperCluster.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MiniZooKeeperCluster.java @@ -54,21 +54,21 @@ public class MiniZooKeeperCluster { private static final int TICK_TIME = 2000; private static final int DEFAULT_CONNECTION_TIMEOUT = 30000; - private int connectionTimeout; + private final int connectionTimeout; private boolean started; /** The default port. If zero, we use a random port. */ private int defaultClientPort = 0; - private List standaloneServerFactoryList; - private List zooKeeperServers; - private List clientPortList; + private final List standaloneServerFactoryList; + private final List zooKeeperServers; + private final List clientPortList; private int activeZKServerIndex; private int tickTime = 0; - private Configuration configuration; + private final Configuration configuration; public MiniZooKeeperCluster() { this(new Configuration()); @@ -96,6 +96,7 @@ public class MiniZooKeeperCluster { /** * Get the list of client ports. + * * @return clientPortList the client port list */ @VisibleForTesting @@ -141,7 +142,8 @@ public class MiniZooKeeperCluster { } } // Make sure that the port is unused. - while (true) { + // break when an unused port is found + do { for (i = 0; i < clientPortList.size(); i++) { if (returnClientPort == clientPortList.get(i)) { // Already used. Update the port and retry. @@ -149,10 +151,7 @@ public class MiniZooKeeperCluster { break; } } - if (i == clientPortList.size()) { - break; // found a unused port, exit - } - } + } while (i != clientPortList.size()); return returnClientPort; } @@ -161,7 +160,7 @@ public class MiniZooKeeperCluster { } public int getBackupZooKeeperServerNum() { - return zooKeeperServers.size()-1; + return zooKeeperServers.size() - 1; } public int getZooKeeperServerNum() { @@ -177,7 +176,7 @@ public class MiniZooKeeperCluster { System.setProperty("zookeeper.preAllocSize", "100"); FileTxnLog.setPreallocSize(100 * 1024); // allow all 4 letter words - System.setProperty("zookeeper.4lw.commands.whitelist","*"); + System.setProperty("zookeeper.4lw.commands.whitelist", "*"); } public int startup(File baseDir) throws IOException, InterruptedException { @@ -210,7 +209,7 @@ public class MiniZooKeeperCluster { // running all the ZK servers for (int i = 0; i < numZooKeeperServers; i++) { - File dir = new File(baseDir, "zookeeper_"+i).getAbsoluteFile(); + File dir = new File(baseDir, "zookeeper_" + i).getAbsoluteFile(); createDir(dir); int tickTimeToUse; if (this.tickTime > 0) { @@ -266,8 +265,7 @@ public class MiniZooKeeperCluster { // We have selected a port as a client port. Update clientPortList if necessary. if (clientPortList.size() <= i) { // it is not in the list, add the port clientPortList.add(currentClientPort); - } - else if (clientPortList.get(i) <= 0) { // the list has invalid port, update with valid port + } else if (clientPortList.get(i) <= 0) { // the list has invalid port, update with valid port clientPortList.remove(i); clientPortList.add(i, currentClientPort); } @@ -403,13 +401,10 @@ public class MiniZooKeeperCluster { long start = System.currentTimeMillis(); while (true) { try { - Socket sock = new Socket("localhost", port); - try { + try (Socket sock = new Socket("localhost", port)) { OutputStream outstream = sock.getOutputStream(); outstream.write("stat".getBytes()); outstream.flush(); - } finally { - sock.close(); } } catch (IOException e) { return true; diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java index c23e3d22546..a318c3157e2 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java @@ -79,9 +79,9 @@ public class RecoverableZooKeeper { // An identifier of this process in the cluster private final String identifier; private final byte[] id; - private Watcher watcher; - private int sessionTimeout; - private String quorumServers; + private final Watcher watcher; + private final int sessionTimeout; + private final String quorumServers; public RecoverableZooKeeper(String quorumServers, int sessionTimeout, Watcher watcher, int maxRetries, int retryIntervalMillis, int maxSleepTime) diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAclReset.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAclReset.java index 03a28469c04..377383a61c4 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAclReset.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAclReset.java @@ -67,13 +67,10 @@ public class ZKAclReset extends Configured implements Tool { private static void resetAcls(final Configuration conf, boolean eraseAcls) throws Exception { - ZKWatcher zkw = new ZKWatcher(conf, "ZKAclReset", null); - try { + try (ZKWatcher zkw = new ZKWatcher(conf, "ZKAclReset", null)) { LOG.info((eraseAcls ? "Erase" : "Set") + " HBase ACLs for " + - zkw.getQuorum() + " " + zkw.getZNodePaths().baseZNode); + zkw.getQuorum() + " " + zkw.getZNodePaths().baseZNode); resetAcls(zkw, zkw.getZNodePaths().baseZNode, eraseAcls); - } finally { - zkw.close(); } } @@ -96,13 +93,20 @@ public class ZKAclReset extends Configured implements Tool { public int run(String[] args) throws Exception { boolean eraseAcls = true; - for (int i = 0; i < args.length; ++i) { - if (args[i].equals("-help")) { - printUsageAndExit(); - } else if (args[i].equals("-set-acls")) { - eraseAcls = false; - } else { - printUsageAndExit(); + for (String arg : args) { + switch (arg) { + case "-help": { + printUsageAndExit(); + break; + } + case "-set-acls": { + eraseAcls = false; + break; + } + default: { + printUsageAndExit(); + break; + } } } diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java index a093f615b51..8bc204414f5 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java @@ -35,8 +35,8 @@ import org.apache.zookeeper.KeeperException; */ @InterfaceAudience.Private public class ZKClusterId { - private ZKWatcher watcher; - private Abortable abortable; + private final ZKWatcher watcher; + private final Abortable abortable; private String id; public ZKClusterId(ZKWatcher watcher, Abortable abortable) { diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKLeaderManager.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKLeaderManager.java index 5918e687640..fa26c0c3171 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKLeaderManager.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKLeaderManager.java @@ -45,9 +45,9 @@ public class ZKLeaderManager extends ZKListener { private final Object lock = new Object(); private final AtomicBoolean leaderExists = new AtomicBoolean(); - private String leaderZNode; - private byte[] nodeId; - private Stoppable candidate; + private final String leaderZNode; + private final byte[] nodeId; + private final Stoppable candidate; public ZKLeaderManager(ZKWatcher watcher, String leaderZNode, byte[] identifier, Stoppable candidate) { diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java index facc5ccf77b..3faf799cd63 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java @@ -244,7 +244,7 @@ public final class ZKUtil { private static final Map BASIC_JAAS_OPTIONS = new HashMap<>(); static { String jaasEnvVar = System.getenv("HBASE_JAAS_DEBUG"); - if (jaasEnvVar != null && "true".equalsIgnoreCase(jaasEnvVar)) { + if ("true".equalsIgnoreCase(jaasEnvVar)) { BASIC_JAAS_OPTIONS.put("debug", "true"); } } @@ -351,7 +351,7 @@ public final class ZKUtil { throws KeeperException { try { Stat s = zkw.getRecoverableZooKeeper().exists(znode, zkw); - boolean exists = s != null ? true : false; + boolean exists = s != null; if (exists) { LOG.debug(zkw.prefix("Set watcher on existing znode=" + znode)); } else { @@ -441,8 +441,7 @@ public final class ZKUtil { ZKWatcher zkw, String znode) throws KeeperException { try { - List children = zkw.getRecoverableZooKeeper().getChildren(znode, zkw); - return children; + return zkw.getRecoverableZooKeeper().getChildren(znode, zkw); } catch(KeeperException.NoNodeException ke) { LOG.debug(zkw.prefix("Unable to list children of znode " + znode + " " + "because node does not exist (not an error)")); @@ -1744,9 +1743,12 @@ public final class ZKUtil { sb.append("<>"); } sb.append("\nBackup master addresses:"); - for (String child : listChildrenNoWatch(zkw, - zkw.getZNodePaths().backupMasterAddressesZNode)) { - sb.append("\n ").append(child); + final List backupMasterChildrenNoWatchList = listChildrenNoWatch(zkw, + zkw.getZNodePaths().backupMasterAddressesZNode); + if (backupMasterChildrenNoWatchList != null) { + for (String child : backupMasterChildrenNoWatchList) { + sb.append("\n ").append(child); + } } sb.append("\nRegion server holding hbase:meta: " + new MetaTableLocator().getMetaRegionLocation(zkw)); @@ -1758,8 +1760,12 @@ public final class ZKUtil { + new MetaTableLocator().getMetaRegionLocation(zkw, i)); } sb.append("\nRegion servers:"); - for (String child : listChildrenNoWatch(zkw, zkw.getZNodePaths().rsZNode)) { - sb.append("\n ").append(child); + final List rsChildrenNoWatchList = + listChildrenNoWatch(zkw, zkw.getZNodePaths().rsZNode); + if (rsChildrenNoWatchList != null) { + for (String child : rsChildrenNoWatchList) { + sb.append("\n ").append(child); + } } try { getReplicationZnodesDump(zkw, sb); @@ -1810,31 +1816,33 @@ public final class ZKUtil { // do a ls -r on this znode sb.append("\n").append(replicationZnode).append(": "); List children = ZKUtil.listChildrenNoWatch(zkw, replicationZnode); - Collections.sort(children); - for (String child : children) { - String znode = ZNodePaths.joinZNode(replicationZnode, child); - if (znode.equals(zkw.getZNodePaths().peersZNode)) { - appendPeersZnodes(zkw, znode, sb); - } else if (znode.equals(zkw.getZNodePaths().queuesZNode)) { - appendRSZnodes(zkw, znode, sb); - } else if (znode.equals(zkw.getZNodePaths().hfileRefsZNode)) { - appendHFileRefsZnodes(zkw, znode, sb); + if (children != null) { + Collections.sort(children); + for (String child : children) { + String zNode = ZNodePaths.joinZNode(replicationZnode, child); + if (zNode.equals(zkw.getZNodePaths().peersZNode)) { + appendPeersZnodes(zkw, zNode, sb); + } else if (zNode.equals(zkw.getZNodePaths().queuesZNode)) { + appendRSZnodes(zkw, zNode, sb); + } else if (zNode.equals(zkw.getZNodePaths().hfileRefsZNode)) { + appendHFileRefsZNodes(zkw, zNode, sb); + } } } } - private static void appendHFileRefsZnodes(ZKWatcher zkw, String hfileRefsZnode, + private static void appendHFileRefsZNodes(ZKWatcher zkw, String hFileRefsZNode, StringBuilder sb) throws KeeperException { - sb.append("\n").append(hfileRefsZnode).append(": "); - for (String peerIdZnode : ZKUtil.listChildrenNoWatch(zkw, hfileRefsZnode)) { - String znodeToProcess = ZNodePaths.joinZNode(hfileRefsZnode, peerIdZnode); - sb.append("\n").append(znodeToProcess).append(": "); - List peerHFileRefsZnodes = ZKUtil.listChildrenNoWatch(zkw, znodeToProcess); - int size = peerHFileRefsZnodes.size(); - for (int i = 0; i < size; i++) { - sb.append(peerHFileRefsZnodes.get(i)); - if (i != size - 1) { - sb.append(", "); + sb.append("\n").append(hFileRefsZNode).append(": "); + final List hFileRefChildrenNoWatchList = + ZKUtil.listChildrenNoWatch(zkw, hFileRefsZNode); + if (hFileRefChildrenNoWatchList != null) { + for (String peerIdZNode : hFileRefChildrenNoWatchList) { + String zNodeToProcess = ZNodePaths.joinZNode(hFileRefsZNode, peerIdZNode); + sb.append("\n").append(zNodeToProcess).append(": "); + List peerHFileRefsZNodes = ZKUtil.listChildrenNoWatch(zkw, zNodeToProcess); + if (peerHFileRefsZNodes != null) { + sb.append(String.join(", ", peerHFileRefsZNodes)); } } } @@ -1946,10 +1954,10 @@ public final class ZKUtil { * @return The array of response strings. * @throws IOException When the socket communication fails. */ - public static String[] getServerStats(String server, int timeout) + private static String[] getServerStats(String server, int timeout) throws IOException { String[] sp = server.split(":"); - if (sp == null || sp.length == 0) { + if (sp.length == 0) { return null; } @@ -2085,7 +2093,7 @@ public final class ZKUtil { * @see #logZKTree(ZKWatcher, String) * @throws KeeperException if an unexpected exception occurs */ - protected static void logZKTree(ZKWatcher zkw, String root, String prefix) + private static void logZKTree(ZKWatcher zkw, String root, String prefix) throws KeeperException { List children = ZKUtil.listChildrenNoWatch(zkw, root);