This is an automated email from the ASF dual-hosted git repository.
nihaljain pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new d309e99f0ab HBASE-27966 HBase Master/RS JVM metrics populated
incorrectly (#5323)
d309e99f0ab is described below
commit d309e99f0ab5a95b041aff009ef187d4e3e65daa
Author: Nihal Jain <nihaljain...@gmail.com>
AuthorDate: Mon Aug 21 23:43:53 2023 +0530
HBASE-27966 HBase Master/RS JVM metrics populated incorrectly (#5323)
Signed-off-by: Bryan Beaudreault <bbeaudrea...@apache.org>
Signed-off-by: Duo Zhang <zhang...@apache.org>
Signed-off-by: Reid Chan <reidc...@apache.org>
---
.../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..6c1ec1dc0ac
--- /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.HBaseTestingUtil;
+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 HBaseTestingUtil UTIL = new HBaseTestingUtil();
+ 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());
+ }
+}
