This is an automated email from the ASF dual-hosted git repository. nihaljain pushed a commit to branch branch-2.5 in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.5 by this push: new 8baae886b16 HBASE-27966 HBase Master/RS JVM metrics populated incorrectly (#5323) (#5364) 8baae886b16 is described below commit 8baae886b16b55c6a56835c58694a27c60cbb8be Author: Nihal Jain <nihaljain...@gmail.com> AuthorDate: Tue Aug 29 21:50:20 2023 +0530 HBASE-27966 HBase Master/RS JVM metrics populated incorrectly (#5323) (#5364) Signed-off-by: Bryan Beaudreault <bbeaudrea...@apache.org> Signed-off-by: Duo Zhang <zhang...@apache.org> Signed-off-by: Reid Chan <reidc...@apache.org> (cherry picked from commit 2713a3ccb380483bab855f242f03d7ca453a3024) --- .../java/org/apache/hadoop/hbase/io/MetricsIO.java | 21 +++- .../org/apache/hadoop/hbase/io/hfile/HFile.java | 10 +- .../hadoop/hbase/regionserver/TestMetricsJvm.java | 113 +++++++++++++++++++++ 3 files changed, 136 insertions(+), 8 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/MetricsIO.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/MetricsIO.java index c2197cef945..58e6f7d01b7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/MetricsIO.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/MetricsIO.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hbase.io; +import com.google.errorprone.annotations.RestrictedApi; import org.apache.hadoop.hbase.CompatibilitySingletonFactory; import org.apache.hadoop.hbase.regionserver.MetricsRegionServerSourceFactory; import org.apache.yetus.audience.InterfaceAudience; @@ -24,10 +25,13 @@ import org.apache.yetus.audience.InterfaceAudience; @InterfaceAudience.Private public class MetricsIO { + private static volatile MetricsIO instance; private final MetricsIOSource source; private final MetricsIOWrapper wrapper; - public MetricsIO(MetricsIOWrapper wrapper) { + @RestrictedApi(explanation = "Should only be called in TestMetricsIO", link = "", + allowedOnPath = ".*/(MetricsIO|TestMetricsIO).java") + MetricsIO(MetricsIOWrapper wrapper) { this(CompatibilitySingletonFactory.getInstance(MetricsRegionServerSourceFactory.class) .createIO(wrapper), wrapper); } @@ -37,6 +41,21 @@ public class MetricsIO { this.wrapper = wrapper; } + /** + * Get a static instance for the MetricsIO so that accessors access the same instance. We want to + * lazy initialize so that correct process name is in place. See HBASE-27966 for more details. + */ + public static MetricsIO getInstance() { + if (instance == null) { + synchronized (MetricsIO.class) { + if (instance == null) { + instance = new MetricsIO(new MetricsIOWrapperImpl()); + } + } + } + return instance; + } + public MetricsIOSource getMetricsSource() { return source; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java index 73346e8ae4a..207c9986651 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java @@ -37,7 +37,6 @@ import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.io.FSDataInputStreamWrapper; import org.apache.hadoop.hbase.io.MetricsIO; -import org.apache.hadoop.hbase.io.MetricsIOWrapperImpl; import org.apache.hadoop.hbase.io.compress.Compression; import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding; import org.apache.hadoop.hbase.io.hfile.ReaderContext.ReaderType; @@ -168,9 +167,6 @@ public final class HFile { // For tests. Gets incremented when we read a block whether from HDFS or from Cache. public static final LongAdder DATABLOCK_READ_COUNT = new LongAdder(); - /** Static instance for the metrics so that HFileReaders access the same instance */ - static final MetricsIO metrics = new MetricsIO(new MetricsIOWrapperImpl()); - /** * Shutdown constructor. */ @@ -193,14 +189,14 @@ public final class HFile { public static final void updateReadLatency(long latencyMillis, boolean pread) { if (pread) { - metrics.updateFsPreadTime(latencyMillis); + MetricsIO.getInstance().updateFsPreadTime(latencyMillis); } else { - metrics.updateFsReadTime(latencyMillis); + MetricsIO.getInstance().updateFsReadTime(latencyMillis); } } public static final void updateWriteLatency(long latencyMillis) { - metrics.updateFsWriteTime(latencyMillis); + MetricsIO.getInstance().updateFsWriteTime(latencyMillis); } /** API required to write an {@link HFile} */ diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsJvm.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsJvm.java new file mode 100644 index 00000000000..049bf12c5a8 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsJvm.java @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.regionserver; + +import static junit.framework.TestCase.assertFalse; +import static junit.framework.TestCase.assertTrue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.testclassification.MetricsTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Pair; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.util.EntityUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({ MetricsTests.class, SmallTests.class }) +public class TestMetricsJvm { + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestMetricsJvm.class); + + private final static HBaseTestingUtility UTIL = new HBaseTestingUtility(); + private static Configuration conf; + + @BeforeClass + public static void before() throws Exception { + conf = UTIL.getConfiguration(); + // The master info server does not run in tests by default. + // Set it to ephemeral port so that it will start + conf.setInt(HConstants.MASTER_INFO_PORT, 0); + UTIL.startMiniCluster(); + } + + @AfterClass + public static void after() throws Exception { + UTIL.shutdownMiniCluster(); + } + + @Test + public void testJvmMetrics() throws Exception { + final Pair<Integer, String> jmxPage = getJmxPage("?qry=Hadoop:service=HBase,name=JvmMetrics*"); + assertNotNull(jmxPage); + + final Integer responseCode = jmxPage.getFirst(); + final String responseBody = jmxPage.getSecond(); + + assertEquals(HttpURLConnection.HTTP_OK, responseCode.intValue()); + assertNotNull(responseBody); + + assertNotFind("\"tag.ProcessName\"\\s*:\\s*\"IO\"", responseBody); + assertReFind("\"tag.ProcessName\"\\s*:\\s*\"Master\"", responseBody); + } + + private Pair<Integer, String> getJmxPage(String query) throws Exception { + URL url = new URL("http://localhost:" + + UTIL.getHBaseCluster().getMaster().getInfoServer().getPort() + "/jmx" + query); + return getUrlContent(url); + } + + private Pair<Integer, String> getUrlContent(URL url) throws Exception { + try (CloseableHttpClient client = HttpClients.createDefault()) { + CloseableHttpResponse resp = client.execute(new HttpGet(url.toURI())); + int code = resp.getStatusLine().getStatusCode(); + if (code == HttpURLConnection.HTTP_OK) { + return new Pair<>(code, EntityUtils.toString(resp.getEntity())); + } + return new Pair<>(code, null); + } + } + + private void assertReFind(String re, String value) { + Pattern p = Pattern.compile(re); + Matcher m = p.matcher(value); + assertTrue("'" + p + "' does not match " + value, m.find()); + } + + private void assertNotFind(String re, String value) { + Pattern p = Pattern.compile(re); + Matcher m = p.matcher(value); + assertFalse("'" + p + "' should not match " + value, m.find()); + } +}