From 17cd8d0c1786d6e3ea5fe7c90b176381db6f9c36 Mon Sep 17 00:00:00 2001 From: Zhe Zhang Date: Tue, 24 Oct 2017 15:06:50 -0700 Subject: [PATCH] Revert "HDFS-12502. nntop should support a category based on FilesInGetListingOps." This reverts commit 60bfee270ed3a653c44c0bc92396167b5022df6e. --- .../hdfs/server/namenode/FSNamesystem.java | 5 ---- .../namenode/top/metrics/TopMetrics.java | 30 +++++-------------- .../namenode/metrics/TestTopMetrics.java | 11 ++----- 3 files changed, 10 insertions(+), 36 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 1e8f319402f..e8d7161d158 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -3673,11 +3673,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, readUnlock(operationName); } logAuditEvent(true, operationName, src); - if (topConf.isEnabled && isAuditEnabled() && isExternalInvocation() - && dl != null && Server.getRemoteUser() != null) { - topMetrics.reportFilesInGetListing(Server.getRemoteUser().toString(), - dl.getPartialListing().length); - } return dl; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/top/metrics/TopMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/top/metrics/TopMetrics.java index 3d8dd19490e..2719c8857ee 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/top/metrics/TopMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/top/metrics/TopMetrics.java @@ -70,14 +70,6 @@ public class TopMetrics implements MetricsSource { public static final Logger LOG = LoggerFactory.getLogger(TopMetrics.class); public static final String TOPMETRICS_METRICS_SOURCE_NAME = "NNTopUserOpCounts"; - /** - * In addition to counts of different RPC calls, NNTop also reports top - * users listing large directories (measured by the number of files involved - * in listing operations from the user). This is important because the CPU - * and GC overhead of a listing operation grows linearly with the number of - * files involved. This category in NNTop is {@link #FILES_IN_GETLISTING}. - */ - public static final String FILES_IN_GETLISTING = "filesInGetListing"; private final boolean isMetricsSourceEnabled; private static void logConf(Configuration conf) { @@ -131,30 +123,22 @@ public class TopMetrics implements MetricsSource { public void report(boolean succeeded, String userName, InetAddress addr, String cmd, String src, String dst, FileStatus status) { // currently nntop only makes use of the username and the command - report(userName, cmd, 1); + report(userName, cmd); } - public void reportFilesInGetListing(String userName, int numFiles) { - report(userName, FILES_IN_GETLISTING, numFiles); - } - - public void report(String userName, String cmd, int delta) { + public void report(String userName, String cmd) { long currTime = Time.monotonicNow(); - report(currTime, userName, cmd, delta); + report(currTime, userName, cmd); } - public void report(long currTime, String userName, String cmd, int delta) { + public void report(long currTime, String userName, String cmd) { LOG.debug("a metric is reported: cmd: {} user: {}", cmd, userName); userName = UserGroupInformation.trimLoginMethod(userName); for (RollingWindowManager rollingWindowManager : rollingWindowManagers .values()) { - rollingWindowManager.recordMetric(currTime, cmd, userName, delta); - // Increase the number of all RPC calls by the user, unless the report - // is for the number of files in a listing operation. - if (!cmd.equals(FILES_IN_GETLISTING)) { - rollingWindowManager.recordMetric(currTime, - TopConf.ALL_CMDS, userName, delta); - } + rollingWindowManager.recordMetric(currTime, cmd, userName, 1); + rollingWindowManager.recordMetric(currTime, + TopConf.ALL_CMDS, userName, 1); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestTopMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestTopMetrics.java index cda4b1466fb..4d3a4f030e2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestTopMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestTopMetrics.java @@ -26,7 +26,6 @@ import org.apache.hadoop.metrics2.lib.Interns; import org.junit.Test; import static org.apache.hadoop.hdfs.server.namenode.top.metrics.TopMetrics.TOPMETRICS_METRICS_SOURCE_NAME; -import static org.apache.hadoop.hdfs.server.namenode.top.metrics.TopMetrics.FILES_IN_GETLISTING; import static org.apache.hadoop.test.MetricsAsserts.getMetrics; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -42,10 +41,9 @@ public class TestTopMetrics { TopMetrics topMetrics = new TopMetrics(conf, topConf.nntopReportingPeriodsMs); // Dummy command - topMetrics.report("test", "listStatus", 1); - topMetrics.report("test", "listStatus", 1); - topMetrics.report("test", "listStatus", 1); - topMetrics.report("test", FILES_IN_GETLISTING, 1000); + topMetrics.report("test", "listStatus"); + topMetrics.report("test", "listStatus"); + topMetrics.report("test", "listStatus"); MetricsRecordBuilder rb = getMetrics(topMetrics); MetricsCollector mc = rb.parent(); @@ -61,8 +59,5 @@ public class TestTopMetrics { verify(rb, times(3)).addCounter(Interns.info("op=listStatus." + "user=test.count", "Total operations performed by user"), 3L); - - verify(rb, times(3)).addCounter(Interns.info("op=" + FILES_IN_GETLISTING + - ".user=test.count", "Total operations performed by user"), 1000L); } }