From 3dd425abfa7a2c458a54b6de1f502e33bcd44e20 Mon Sep 17 00:00:00 2001 From: Qi Yu Date: Wed, 25 Nov 2020 15:13:09 +0800 Subject: [PATCH] HBASE-25323 Fix potential NPE when the zookeeper path of RegionServerTracker does not exist when start (#2702) Signed-off-by: Duo Zhang Signed-off-by: Guanghao Zhang --- .../hbase/master/RegionServerTracker.java | 28 +++++++++++-------- .../hbase/zookeeper/MasterAddressTracker.java | 2 +- .../apache/hadoop/hbase/zookeeper/ZKUtil.java | 5 ++-- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionServerTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionServerTracker.java index 9d33a212085..336f9dc04f8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionServerTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionServerTracker.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master; import java.io.IOException; import java.io.InterruptedIOException; +import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -35,6 +36,7 @@ import org.apache.hadoop.hbase.zookeeper.ZKListener; import org.apache.hadoop.hbase.zookeeper.ZKUtil; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; import org.apache.hadoop.hbase.zookeeper.ZNodePaths; +import org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils; import org.apache.yetus.audience.InterfaceAudience; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; @@ -129,21 +131,24 @@ public class RegionServerTracker extends ZKListener { splittingServersFromWALDir.stream().filter(s -> !deadServersFromPE.contains(s)). forEach(s -> LOG.error("{} has no matching ServerCrashProcedure", s)); //create ServerNode for all possible live servers from wal directory - liveServersFromWALDir.stream() + liveServersFromWALDir .forEach(sn -> server.getAssignmentManager().getRegionStates().getOrCreateServer(sn)); watcher.registerListener(this); synchronized (this) { List servers = ZKUtil.listChildrenAndWatchForNewChildren(watcher, watcher.getZNodePaths().rsZNode); - for (String n : servers) { - Pair pair = getServerInfo(n); - ServerName serverName = pair.getFirst(); - RegionServerInfo info = pair.getSecond(); - regionServers.add(serverName); - ServerMetrics serverMetrics = info != null ? ServerMetricsBuilder.of(serverName, - VersionInfoUtil.getVersionNumber(info.getVersionInfo()), - info.getVersionInfo().getVersion()) : ServerMetricsBuilder.of(serverName); - serverManager.checkAndRecordNewServer(serverName, serverMetrics); + if (null != servers) { + for (String n : servers) { + Pair pair = getServerInfo(n); + ServerName serverName = pair.getFirst(); + RegionServerInfo info = pair.getSecond(); + regionServers.add(serverName); + ServerMetrics serverMetrics = info != null ? + ServerMetricsBuilder.of(serverName, VersionInfoUtil.getVersionNumber(info.getVersionInfo()), + info.getVersionInfo().getVersion()) : + ServerMetricsBuilder.of(serverName); + serverManager.checkAndRecordNewServer(serverName, serverMetrics); + } } serverManager.findDeadServersAndProcess(deadServersFromPE, liveServersFromWALDir); } @@ -163,8 +168,9 @@ public class RegionServerTracker extends ZKListener { server.abort("Unexpected zk exception getting RS nodes", e); return; } - Set servers = + Set servers = CollectionUtils.isEmpty(names) ? Collections.emptySet() : names.stream().map(ServerName::parseServerName).collect(Collectors.toSet()); + for (Iterator iter = regionServers.iterator(); iter.hasNext();) { ServerName sn = iter.next(); if (!servers.contains(sn)) { diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java index 320623822ec..133d966df86 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java @@ -297,7 +297,7 @@ public class MasterAddressTracker extends ZKNodeTracker { public static List getBackupMastersAndRenewWatch( ZKWatcher zkw) throws InterruptedIOException { // Build Set of backup masters from ZK nodes - List backupMasterStrings = Collections.emptyList(); + List backupMasterStrings = null; try { backupMasterStrings = ZKUtil.listChildrenAndWatchForNewChildren(zkw, zkw.getZNodePaths().backupMasterAddressesZNode); 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 45732d2efdd..25320080015 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 @@ -445,16 +445,15 @@ public final class ZKUtil { } catch(KeeperException.NoNodeException ke) { LOG.debug(zkw.prefix("Unable to list children of znode " + znode + " " + "because node does not exist (not an error)")); - return null; } catch (KeeperException e) { LOG.warn(zkw.prefix("Unable to list children of znode " + znode + " "), e); zkw.keeperException(e); - return null; } catch (InterruptedException e) { LOG.warn(zkw.prefix("Unable to list children of znode " + znode + " "), e); zkw.interruptedException(e); - return null; } + + return null; } /**