From a5d9aaad12b1ac7eede0fd9a84b2ad673e0c4725 Mon Sep 17 00:00:00 2001 From: Chris Nauroth Date: Wed, 2 Oct 2013 05:37:21 +0000 Subject: [PATCH] HDFS-5279. Merging change r1528308 from trunk to branch-2. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1528310 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 + .../server/namenode/NamenodeJspHelper.java | 49 ++++++++++--- .../src/main/webapps/hdfs/corrupt_files.jsp | 10 ++- .../src/main/webapps/hdfs/dfshealth.jsp | 13 +--- .../src/main/webapps/hdfs/dfsnodelist.jsp | 4 +- .../namenode/TestNameNodeJspHelper.java | 73 +++++++++++++++++++ 6 files changed, 125 insertions(+), 27 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index ad15f976682..0cef920e227 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -144,6 +144,9 @@ Release 2.1.2 - UNRELEASED HDFS-5255. Distcp job fails with hsftp when https is enabled in insecure cluster. (Arpit Agarwal) + HDFS-5279. Guard against NullPointerException in NameNode JSP pages before + initialization of FSNamesystem. (cnauroth) + Release 2.1.1-beta - 2013-09-23 INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java index 461dacc986b..147feb71f3a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java @@ -30,6 +30,7 @@ import java.net.URLEncoder; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -209,6 +210,9 @@ class NamenodeJspHelper { static void generateSnapshotReport(JspWriter out, FSNamesystem fsn) throws IOException { + if (fsn == null) { + return; + } out.println("
" + "\n" + "" @@ -651,7 +655,8 @@ class NamenodeJspHelper { .getAttribute(JspHelper.CURRENT_CONF); // We can't redirect if there isn't a DN to redirect to. // Lets instead show a proper error message. - if (nn.getNamesystem().getNumLiveDataNodes() < 1) { + FSNamesystem fsn = nn.getNamesystem(); + if (fsn == null || fsn.getNumLiveDataNodes() < 1) { throw new IOException("Can't browse the DFS since there are no " + "live nodes available to redirect to."); } @@ -687,6 +692,20 @@ class NamenodeJspHelper { resp.sendRedirect(redirectLocation); } + /** + * Returns a descriptive label for the running NameNode. If the NameNode has + * initialized to the point of running its RPC server, then this label consists + * of the host and port of the RPC server. Otherwise, the label is a message + * stating that the NameNode is still initializing. + * + * @param nn NameNode to describe + * @return String NameNode label + */ + static String getNameNodeLabel(NameNode nn) { + return nn.getRpcServer() != null ? nn.getNameNodeAddressHostPortString() : + "initializing"; + } + static class NodeListJsp { private int rowNum = 0; @@ -841,6 +860,9 @@ class NamenodeJspHelper { HttpServletRequest request) throws IOException { final NameNode nn = NameNodeHttpServer.getNameNodeFromContext(context); final FSNamesystem ns = nn.getNamesystem(); + if (ns == null) { + return; + } final DatanodeManager dm = ns.getBlockManager().getDatanodeManager(); final List live = new ArrayList(); @@ -1018,14 +1040,16 @@ class NamenodeJspHelper { final BlockManager blockManager; XMLBlockInfo(FSNamesystem fsn, Long blockId) { - this.blockManager = fsn.getBlockManager(); + this.blockManager = fsn != null ? fsn.getBlockManager() : null; if (blockId == null) { this.block = null; this.inode = null; } else { this.block = new Block(blockId); - this.inode = ((INode)blockManager.getBlockCollection(block)).asFile(); + this.inode = blockManager != null ? + ((INode)blockManager.getBlockCollection(block)).asFile() : + null; } } @@ -1099,8 +1123,10 @@ class NamenodeJspHelper { } doc.startTag("replicas"); - for(final Iterator it = blockManager.datanodeIterator(block); - it.hasNext(); ) { + for (final Iterator it = blockManager != null ? + blockManager.datanodeIterator(block) : + Collections.emptyList().iterator(); + it.hasNext();) { doc.startTag("replica"); DatanodeDescriptor dd = it.next(); @@ -1136,7 +1162,7 @@ class NamenodeJspHelper { XMLCorruptBlockInfo(FSNamesystem fsn, Configuration conf, int numCorruptBlocks, Long startingBlockId) { - this.blockManager = fsn.getBlockManager(); + this.blockManager = fsn != null ? fsn.getBlockManager() : null; this.conf = conf; this.numCorruptBlocks = numCorruptBlocks; this.startingBlockId = startingBlockId; @@ -1159,16 +1185,19 @@ class NamenodeJspHelper { doc.endTag(); doc.startTag("num_missing_blocks"); - doc.pcdata(""+blockManager.getMissingBlocksCount()); + doc.pcdata("" + (blockManager != null ? + blockManager.getMissingBlocksCount() : 0)); doc.endTag(); doc.startTag("num_corrupt_replica_blocks"); - doc.pcdata(""+blockManager.getCorruptReplicaBlocksCount()); + doc.pcdata("" + (blockManager != null ? + blockManager.getCorruptReplicaBlocksCount() : 0)); doc.endTag(); doc.startTag("corrupt_replica_block_ids"); - final long[] corruptBlockIds = blockManager.getCorruptReplicaBlockIds( - numCorruptBlocks, startingBlockId); + final long[] corruptBlockIds = blockManager != null ? + blockManager.getCorruptReplicaBlockIds(numCorruptBlocks, + startingBlockId) : null; if (corruptBlockIds != null) { for (Long blockId: corruptBlockIds) { doc.startTag("block_id"); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/corrupt_files.jsp b/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/corrupt_files.jsp index 04820754dc5..7c9050ddb1c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/corrupt_files.jsp +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/corrupt_files.jsp @@ -25,6 +25,7 @@ import="org.apache.hadoop.fs.Path" import="org.apache.hadoop.ha.HAServiceProtocol.HAServiceState" import="java.util.Collection" + import="java.util.Collections" import="java.util.Arrays" %> <%!//for java.io.Serializable private static final long serialVersionUID = 1L;%> @@ -34,9 +35,10 @@ HAServiceState nnHAState = nn.getServiceState(); boolean isActive = (nnHAState == HAServiceState.ACTIVE); String namenodeRole = nn.getRole().toString(); - String namenodeLabel = nn.getNameNodeAddressHostPortString(); - Collection corruptFileBlocks = - fsn.listCorruptFileBlocks("/", null); + String namenodeLabel = NamenodeJspHelper.getNameNodeLabel(nn); + Collection corruptFileBlocks = fsn != null ? + fsn.listCorruptFileBlocks("/", null) : + Collections.emptyList(); int corruptFileCount = corruptFileBlocks.size(); %> @@ -48,7 +50,7 @@

<%=namenodeRole%> '<%=namenodeLabel%>'

<%=NamenodeJspHelper.getVersionTable(fsn)%>
-<% if (isActive) { %> +<% if (isActive && fsn != null) { %> Browse the filesystem
<% } %> diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfshealth.jsp b/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfshealth.jsp index 36f6199d978..10872a7af09 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfshealth.jsp +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfshealth.jsp @@ -34,29 +34,20 @@ boolean isActive = (nnHAState == HAServiceState.ACTIVE); String namenodeRole = nn.getRole().toString(); String namenodeState = nnHAState.toString(); - String namenodeLabel = nn.getRpcServer() != null ? - nn.getNameNodeAddressHostPortString() : null; + String namenodeLabel = NamenodeJspHelper.getNameNodeLabel(nn); %> -<% if (namenodeLabel != null) { %> Hadoop <%=namenodeRole%> <%=namenodeLabel%> -<% } else { %> -Hadoop <%=namenodeRole%> -<% } %> -<% if (namenodeLabel != null) { %>

<%=namenodeRole%> '<%=namenodeLabel%>' (<%=namenodeState%>)

-<% } else { %> -

<%=namenodeRole%> (<%=namenodeState%>)

-<% } %> <%= NamenodeJspHelper.getVersionTable(fsn) %>
-<% if (isActive) { %> +<% if (isActive && fsn != null) { %> Browse the filesystem
<% } %> <%=namenodeRole%> Logs diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfsnodelist.jsp b/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfsnodelist.jsp index 446104a0af5..3bb34986038 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfsnodelist.jsp +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/dfsnodelist.jsp @@ -33,7 +33,7 @@ String namenodeRole = nn.getRole().toString(); FSNamesystem fsn = nn.getNamesystem(); HAServiceState nnHAState = nn.getServiceState(); boolean isActive = (nnHAState == HAServiceState.ACTIVE); -String namenodeLabel = nn.getNameNodeAddressHostPortString(); +String namenodeLabel = NamenodeJspHelper.getNameNodeLabel(nn); %> @@ -46,7 +46,7 @@ String namenodeLabel = nn.getNameNodeAddressHostPortString();

<%=namenodeRole%> '<%=namenodeLabel%>'

<%= NamenodeJspHelper.getVersionTable(fsn) %>
-<% if (isActive) { %> +<% if (isActive && fsn != null) { %> Browse the filesystem
<% } %> <%=namenodeRole%> Logs
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeJspHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeJspHelper.java index a847a9438a3..3207f0ccb01 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeJspHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeJspHelper.java @@ -25,12 +25,15 @@ import static org.apache.hadoop.hdfs.server.namenode.startupprogress.Phase.SAVIN import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.io.IOException; import java.util.List; import java.util.regex.Pattern; +import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import javax.servlet.jsp.JspWriter; import org.apache.hadoop.conf.Configuration; @@ -45,6 +48,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.znerd.xmlenc.XMLOutputter; public class TestNameNodeJspHelper { @@ -117,6 +121,75 @@ public class TestNameNodeJspHelper { Assert.assertEquals("", NamenodeJspHelper.getRollingUpgradeText(null)); } + /** + * Tests for non-null, non-empty NameNode label. + */ + @Test + public void testGetNameNodeLabel() { + String nameNodeLabel = NamenodeJspHelper.getNameNodeLabel( + cluster.getNameNode()); + Assert.assertNotNull(nameNodeLabel); + Assert.assertFalse(nameNodeLabel.isEmpty()); + } + + /** + * Tests for non-null, non-empty NameNode label when called before + * initialization of the NameNode RPC server. + */ + @Test + public void testGetNameNodeLabelNullRpcServer() { + NameNode nn = mock(NameNode.class); + when(nn.getRpcServer()).thenReturn(null); + String nameNodeLabel = NamenodeJspHelper.getNameNodeLabel( + cluster.getNameNode()); + Assert.assertNotNull(nameNodeLabel); + Assert.assertFalse(nameNodeLabel.isEmpty()); + } + + /** + * Tests that passing a null FSNamesystem to generateSnapshotReport does not + * throw NullPointerException. + */ + @Test + public void testGenerateSnapshotReportNullNamesystem() throws Exception { + NamenodeJspHelper.generateSnapshotReport(mock(JspWriter.class), null); + } + + /** + * Tests that redirectToRandomDataNode does not throw NullPointerException if + * it finds a null FSNamesystem. + */ + @Test(expected=IOException.class) + public void testRedirectToRandomDataNodeNullNamesystem() throws Exception { + NameNode nn = mock(NameNode.class); + when(nn.getNamesystem()).thenReturn(null); + ServletContext context = mock(ServletContext.class); + when(context.getAttribute("name.node")).thenReturn(nn); + NamenodeJspHelper.redirectToRandomDataNode(context, + mock(HttpServletRequest.class), mock(HttpServletResponse.class)); + } + + /** + * Tests that XMLBlockInfo does not throw NullPointerException if it finds a + * null FSNamesystem. + */ + @Test + public void testXMLBlockInfoNullNamesystem() throws IOException { + XMLOutputter doc = new XMLOutputter(mock(JspWriter.class), "UTF-8"); + new NamenodeJspHelper.XMLBlockInfo(null, 1L).toXML(doc); + } + + /** + * Tests that XMLCorruptBlockInfo does not throw NullPointerException if it + * finds a null FSNamesystem. + */ + @Test + public void testXMLCorruptBlockInfoNullNamesystem() throws IOException { + XMLOutputter doc = new XMLOutputter(mock(JspWriter.class), "UTF-8"); + new NamenodeJspHelper.XMLCorruptBlockInfo(null, mock(Configuration.class), + 10, 1L).toXML(doc); + } + /** * Checks if the list contains any string that partially matches the regex. *
Snapshottable directories