From a505876bdaf49e2011b506d054504b46898cfc71 Mon Sep 17 00:00:00 2001 From: Yiqun Lin Date: Mon, 17 Dec 2018 12:35:07 +0800 Subject: [PATCH] HDFS-13869. RBF: Handle NPE for NamenodeBeanMetrics#getFederationMetrics. Contributed by Ranith Sardar. --- .../metrics/NamenodeBeanMetrics.java | 149 +++++++++++++++--- .../hdfs/server/federation/router/Router.java | 8 +- .../server/federation/router/TestRouter.java | 14 ++ 3 files changed, 147 insertions(+), 24 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/NamenodeBeanMetrics.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/NamenodeBeanMetrics.java index 4380ae9eebb..a05fdc14498 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/NamenodeBeanMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/NamenodeBeanMetrics.java @@ -168,8 +168,12 @@ public class NamenodeBeanMetrics } } - private FederationMetrics getFederationMetrics() { - return this.router.getMetrics(); + private FederationMetrics getFederationMetrics() throws IOException { + FederationMetrics metrics = getRouter().getMetrics(); + if (metrics == null) { + throw new IOException("Federated metrics is not initialized"); + } + return metrics; } ///////////////////////////////////////////////////////// @@ -188,22 +192,42 @@ public class NamenodeBeanMetrics @Override public long getUsed() { - return getFederationMetrics().getUsedCapacity(); + try { + return getFederationMetrics().getUsedCapacity(); + } catch (IOException e) { + LOG.debug("Failed to get the used capacity", e.getMessage()); + } + return 0; } @Override public long getFree() { - return getFederationMetrics().getRemainingCapacity(); + try { + return getFederationMetrics().getRemainingCapacity(); + } catch (IOException e) { + LOG.debug("Failed to get remaining capacity", e.getMessage()); + } + return 0; } @Override public long getTotal() { - return getFederationMetrics().getTotalCapacity(); + try { + return getFederationMetrics().getTotalCapacity(); + } catch (IOException e) { + LOG.debug("Failed to Get total capacity", e.getMessage()); + } + return 0; } @Override public long getProvidedCapacity() { - return getFederationMetrics().getProvidedSpace(); + try { + return getFederationMetrics().getProvidedSpace(); + } catch (IOException e) { + LOG.debug("Failed to get provided capacity", e.getMessage()); + } + return 0; } @Override @@ -261,39 +285,79 @@ public class NamenodeBeanMetrics @Override public long getTotalBlocks() { - return getFederationMetrics().getNumBlocks(); + try { + return getFederationMetrics().getNumBlocks(); + } catch (IOException e) { + LOG.debug("Failed to get number of blocks", e.getMessage()); + } + return 0; } @Override public long getNumberOfMissingBlocks() { - return getFederationMetrics().getNumOfMissingBlocks(); + try { + return getFederationMetrics().getNumOfMissingBlocks(); + } catch (IOException e) { + LOG.debug("Failed to get number of missing blocks", e.getMessage()); + } + return 0; } @Override @Deprecated public long getPendingReplicationBlocks() { - return getFederationMetrics().getNumOfBlocksPendingReplication(); + try { + return getFederationMetrics().getNumOfBlocksPendingReplication(); + } catch (IOException e) { + LOG.debug("Failed to get number of blocks pending replica", + e.getMessage()); + } + return 0; } @Override public long getPendingReconstructionBlocks() { - return getFederationMetrics().getNumOfBlocksPendingReplication(); + try { + return getFederationMetrics().getNumOfBlocksPendingReplication(); + } catch (IOException e) { + LOG.debug("Failed to get number of blocks pending replica", + e.getMessage()); + } + return 0; } @Override @Deprecated public long getUnderReplicatedBlocks() { - return getFederationMetrics().getNumOfBlocksUnderReplicated(); + try { + return getFederationMetrics().getNumOfBlocksUnderReplicated(); + } catch (IOException e) { + LOG.debug("Failed to get number of blocks under replicated", + e.getMessage()); + } + return 0; } @Override public long getLowRedundancyBlocks() { - return getFederationMetrics().getNumOfBlocksUnderReplicated(); + try { + return getFederationMetrics().getNumOfBlocksUnderReplicated(); + } catch (IOException e) { + LOG.debug("Failed to get number of blocks under replicated", + e.getMessage()); + } + return 0; } @Override public long getPendingDeletionBlocks() { - return getFederationMetrics().getNumOfBlocksPendingDeletion(); + try { + return getFederationMetrics().getNumOfBlocksPendingDeletion(); + } catch (IOException e) { + LOG.debug("Failed to get number of blocks pending deletion", + e.getMessage()); + } + return 0; } @Override @@ -471,7 +535,12 @@ public class NamenodeBeanMetrics @Override public long getNNStartedTimeInMillis() { - return this.router.getStartTime(); + try { + return getRouter().getStartTime(); + } catch (IOException e) { + LOG.debug("Failed to get the router startup time", e.getMessage()); + } + return 0; } @Override @@ -527,7 +596,12 @@ public class NamenodeBeanMetrics @Override public long getFilesTotal() { - return getFederationMetrics().getNumFiles(); + try { + return getFederationMetrics().getNumFiles(); + } catch (IOException e) { + LOG.debug("Failed to get number of files", e.getMessage()); + } + return 0; } @Override @@ -537,12 +611,22 @@ public class NamenodeBeanMetrics @Override public int getNumLiveDataNodes() { - return this.router.getMetrics().getNumLiveNodes(); + try { + return getFederationMetrics().getNumLiveNodes(); + } catch (IOException e) { + LOG.debug("Failed to get number of live nodes", e.getMessage()); + } + return 0; } @Override public int getNumDeadDataNodes() { - return this.router.getMetrics().getNumDeadNodes(); + try { + return getFederationMetrics().getNumDeadNodes(); + } catch (IOException e) { + LOG.debug("Failed to get number of dead nodes", e.getMessage()); + } + return 0; } @Override @@ -552,17 +636,35 @@ public class NamenodeBeanMetrics @Override public int getNumDecomLiveDataNodes() { - return this.router.getMetrics().getNumDecomLiveNodes(); + try { + return getFederationMetrics().getNumDecomLiveNodes(); + } catch (IOException e) { + LOG.debug("Failed to get the number of live decommissioned datanodes", + e.getMessage()); + } + return 0; } @Override public int getNumDecomDeadDataNodes() { - return this.router.getMetrics().getNumDecomDeadNodes(); + try { + return getFederationMetrics().getNumDecomDeadNodes(); + } catch (IOException e) { + LOG.debug("Failed to get the number of dead decommissioned datanodes", + e.getMessage()); + } + return 0; } @Override public int getNumDecommissioningDataNodes() { - return this.router.getMetrics().getNumDecommissioningNodes(); + try { + return getFederationMetrics().getNumDecommissioningNodes(); + } catch (IOException e) { + LOG.debug("Failed to get number of decommissioning nodes", + e.getMessage()); + } + return 0; } @Override @@ -702,4 +804,11 @@ public class NamenodeBeanMetrics public String getVerifyECWithTopologyResult() { return null; } + + private Router getRouter() throws IOException { + if (this.router == null) { + throw new IOException("Router is not initialized"); + } + return this.router; + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/Router.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/Router.java index 32882735ebf..3182e27bcc9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/Router.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/Router.java @@ -586,11 +586,11 @@ public class Router extends CompositeService { * * @return Namenode metrics. */ - public NamenodeBeanMetrics getNamenodeMetrics() { - if (this.metrics != null) { - return this.metrics.getNamenodeMetrics(); + public NamenodeBeanMetrics getNamenodeMetrics() throws IOException { + if (this.metrics == null) { + throw new IOException("Namenode metrics is not initialized"); } - return null; + return this.metrics.getNamenodeMetrics(); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouter.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouter.java index db4be292fd6..f83cfda6015 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouter.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouter.java @@ -203,4 +203,18 @@ public class TestRouter { router.stop(); router.close(); } + + @Test + public void testRouterMetricsWhenDisabled() throws Exception { + + Router router = new Router(); + router.init(new RouterConfigBuilder(conf).rpc().build()); + router.start(); + + intercept(IOException.class, "Namenode metrics is not initialized", + () -> router.getNamenodeMetrics().getCacheCapacity()); + + router.stop(); + router.close(); + } }