From cdefd97521a800eefc462b9f01fc641f2ddbba54 Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Mon, 25 Jul 2022 18:20:29 +0800 Subject: [PATCH 1/4] HDFS-16683. All method metrics related to the rpc protocol should be initialized. --- .../lib/MutableRatesWithAggregation.java | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java index 4c5f0a844aaab..7f776571f85c3 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java @@ -21,7 +21,10 @@ import java.lang.ref.WeakReference; import java.lang.reflect.Method; import java.util.HashSet; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -71,11 +74,20 @@ public void init(Class protocol) { if (protocolCache.contains(protocol)) { return; } - protocolCache.add(protocol); - for (Method method : protocol.getDeclaredMethods()) { - String name = method.getName(); - LOG.debug(name); - addMetricIfNotExists(name); + + List> protocols = new ArrayList<>(); + protocols.add(protocol); + if (protocol.getDeclaredMethods().length == 0) { + protocols.addAll(Arrays.asList(protocol.getInterfaces())); + } + + for (Class pClass : protocols) { + protocolCache.add(pClass); + for (Method method : pClass.getDeclaredMethods()) { + String name = method.getName(); + LOG.debug(name); + addMetricIfNotExists(name); + } } } From 90286d6b3db325d87c966d61c2bcad97dfeec231 Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Sun, 31 Jul 2022 19:45:34 +0800 Subject: [PATCH 2/4] Add a unit test and resolve conflicts. --- .../lib/MutableRatesWithAggregation.java | 22 +++++-------------- .../server/datanode/TestDataNodeMetrics.java | 13 ++++++++++- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java index 7f776571f85c3..60b33a84b519b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableRatesWithAggregation.java @@ -21,10 +21,7 @@ import java.lang.ref.WeakReference; import java.lang.reflect.Method; import java.util.HashSet; -import java.util.ArrayList; -import java.util.Arrays; import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -74,20 +71,11 @@ public void init(Class protocol) { if (protocolCache.contains(protocol)) { return; } - - List> protocols = new ArrayList<>(); - protocols.add(protocol); - if (protocol.getDeclaredMethods().length == 0) { - protocols.addAll(Arrays.asList(protocol.getInterfaces())); - } - - for (Class pClass : protocols) { - protocolCache.add(pClass); - for (Method method : pClass.getDeclaredMethods()) { - String name = method.getName(); - LOG.debug(name); - addMetricIfNotExists(name); - } + protocolCache.add(protocol); + for (Method method : protocol.getMethods()) { + String name = method.getName(); + LOG.debug(name); + addMetricIfNotExists(name); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java index 2bf7861287a2d..1d26b5fcd504a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java @@ -70,6 +70,7 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import javax.management.AttributeNotFoundException; import javax.management.MBeanServer; import javax.management.ObjectName; @@ -591,7 +592,7 @@ public Boolean get() { } @Test - public void testNNRpcMetricsWithNonHA() throws IOException { + public void testNNRpcMetricsWithNonHA() throws Exception { Configuration conf = new HdfsConfiguration(); // setting heartbeat interval to 1 hour to prevent bpServiceActor sends // heartbeat periodically to NN during running test case, and bpServiceActor @@ -599,6 +600,16 @@ public void testNNRpcMetricsWithNonHA() throws IOException { conf.setTimeDuration(DFS_HEARTBEAT_INTERVAL_KEY, 1, TimeUnit.HOURS); MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); cluster.waitActive(); + final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); + final ObjectName mxbeanName = + new ObjectName("Hadoop:service=NameNode," + + "name=RpcDetailedActivityForPort" + + cluster.getNameNode().getNameNodeAddress().getPort()); + try { + mbs.getAttribute(mxbeanName, "CommitBlockSynchronizationNumOps"); + } catch (AttributeNotFoundException e) { + fail("Datanode protocol metrics have not been fully initialized."); + } DataNode dn = cluster.getDataNodes().get(0); MetricsRecordBuilder rb = getMetrics(dn.getMetrics().name()); assertCounter("HeartbeatsNumOps", 1L, rb); From b79ea582bb34751ed8073ec77f2bcb1e9d82038e Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Tue, 2 Aug 2022 21:50:03 +0800 Subject: [PATCH 3/4] Move the unit test to TestNameNodeMetrics. --- .../hdfs/server/datanode/TestDataNodeMetrics.java | 13 +------------ .../namenode/metrics/TestNameNodeMetrics.java | 12 ++++++++++++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java index 1d26b5fcd504a..2bf7861287a2d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java @@ -70,7 +70,6 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import javax.management.AttributeNotFoundException; import javax.management.MBeanServer; import javax.management.ObjectName; @@ -592,7 +591,7 @@ public Boolean get() { } @Test - public void testNNRpcMetricsWithNonHA() throws Exception { + public void testNNRpcMetricsWithNonHA() throws IOException { Configuration conf = new HdfsConfiguration(); // setting heartbeat interval to 1 hour to prevent bpServiceActor sends // heartbeat periodically to NN during running test case, and bpServiceActor @@ -600,16 +599,6 @@ public void testNNRpcMetricsWithNonHA() throws Exception { conf.setTimeDuration(DFS_HEARTBEAT_INTERVAL_KEY, 1, TimeUnit.HOURS); MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); cluster.waitActive(); - final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); - final ObjectName mxbeanName = - new ObjectName("Hadoop:service=NameNode," + - "name=RpcDetailedActivityForPort" + - cluster.getNameNode().getNameNodeAddress().getPort()); - try { - mbs.getAttribute(mxbeanName, "CommitBlockSynchronizationNumOps"); - } catch (AttributeNotFoundException e) { - fail("Datanode protocol metrics have not been fully initialized."); - } DataNode dn = cluster.getDataNodes().get(0); MetricsRecordBuilder rb = getMetrics(dn.getMetrics().name()); assertCounter("HeartbeatsNumOps", 1L, rb); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java index aaedb8288e410..1f828fff8beeb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java @@ -48,6 +48,9 @@ import java.util.EnumSet; import java.util.List; import java.util.Random; + +import org.apache.hadoop.hdfs.server.namenode.NameNodeRpcServer; +import org.apache.hadoop.ipc.metrics.RpcDetailedMetrics; import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableList; import org.slf4j.Logger; @@ -1128,4 +1131,13 @@ public void testEditLogTailing() throws Exception { } } + @Test + public void testNNRPCMetricIntegrity() { + RpcDetailedMetrics metrics = + ((NameNodeRpcServer) cluster.getNameNode() + .getRpcServer()).getClientRpcServer().getRpcDetailedMetrics(); + MetricsRecordBuilder rb = getMetrics(metrics.name()); + // CommitBlockSynchronizationNumOps should exist. + assertCounter("CommitBlockSynchronizationNumOps", 0L, rb); + } } From 448f038518c29c90988310ba5b70961e8ec17184 Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Wed, 3 Aug 2022 12:35:23 +0800 Subject: [PATCH 4/4] Code style. --- .../hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java index 1f828fff8beeb..4d472d74b46a8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java @@ -1131,6 +1131,7 @@ public void testEditLogTailing() throws Exception { } } + @Test public void testNNRPCMetricIntegrity() { RpcDetailedMetrics metrics =