[ https://issues.apache.org/jira/browse/HBASE-13314?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14376375#comment-14376375 ]
Srikanth Srungarapu commented on HBASE-13314: --------------------------------------------- Patch looks good to me. Just a minor nit: As we're already fetching getBackupMastersSize(), can't we just use it for assertion for in if clause? Same for getServersSize() too. {code} + sb.append("\nNumber of backup masters: " + getBackupMastersSize()); + if (backupMasters != null && !backupMasters.isEmpty()) { {code} > Fix NPE in HMaster.getClusterStatus() > ------------------------------------- > > Key: HBASE-13314 > URL: https://issues.apache.org/jira/browse/HBASE-13314 > Project: HBase > Issue Type: Bug > Reporter: Matteo Bertozzi > Assignee: Matteo Bertozzi > Priority: Minor > Attachments: HBASE-13314-v0.patch > > > On a test cluster we got a > {noformat} > java.lang.NullPointerException > at > org.apache.hadoop.hbase.master.HMaster.getClusterStatus(HMaster.java:1742) > at > org.apache.hadoop.hbase.master.balancer.ClusterStatusChore.chore(ClusterStatusChore.java:50) > at org.apache.hadoop.hbase.Chore.run(Chore.java:87) > {noformat} > In HMaster.getClusterStatus() we have a couple of NPE. > listChildrenNoWatch() may return null if the node is not found > {code} > try { > backupMasterStrings = ZKUtil.listChildrenNoWatch(this.zooKeeper, > this.zooKeeper.backupMasterAddressesZNode); <--- THIS MAY RETURN NULL > } catch (KeeperException e) { > LOG.warn(this.zooKeeper.prefix("Unable to list backup servers"), e); > backupMasterStrings = new ArrayList<String>(0); > } > List<ServerName> backupMasters = new ArrayList<ServerName>( > backupMasterStrings.size()); <--- WE DON'T CHECK FOR NULL > {code} > then below, we build ClusterStatus with args that may be null > {code} > String clusterId = fileSystemManager != null ? > fileSystemManager.getClusterId().toString() : null; > Map<String, RegionState> regionsInTransition = assignmentManager != null ? > assignmentManager.getRegionStates().getRegionsInTransition() : null; > String[] coprocessors = cpHost != null ? getMasterCoprocessors() : null; > Map<ServerName, ServerLoad> onlineServers = null; > Set<ServerName> deadServers = null; > if (serverManager != null) { > deadServers = serverManager.getDeadServers().copyServerNames(); > onlineServers = serverManager.getOnlineServers(); > } > return new ClusterStatus(VersionInfo.getVersion(), clusterId, > onlineServers, deadServers, serverName, backupMasters, > regionsInTransition, coprocessors, balancerOn); > {code} > In ClusterStatus equals(), hashCode(), toString() we don't check for nulls -- This message was sent by Atlassian JIRA (v6.3.4#6332)