From bbd7280f7d23471ba2f5775f67ff7426a8432391 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Sun, 25 Jun 2023 22:32:52 +0200 Subject: [PATCH 1/5] HDDS-8923. Expose XceiverClient cache stats as metrics --- .../apache/hadoop/hdds/scm/XceiverClientManager.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java index d5ee5af8c98f..ab9c0ef1d5da 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java @@ -46,6 +46,8 @@ import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE; import static org.apache.hadoop.hdds.conf.ConfigTag.PERFORMANCE; import static org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes.NO_REPLICA_FOUND; + +import org.apache.hadoop.util.CacheMetrics; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -68,6 +70,7 @@ public class XceiverClientManager implements Closeable, XceiverClientFactory { private final ConfigurationSource conf; private final ScmClientConfig clientConfig; private final Cache clientCache; + private final CacheMetrics cacheMetrics; private List caCerts; private static XceiverClientMetrics metrics; @@ -99,6 +102,7 @@ public XceiverClientManager(ConfigurationSource conf, } this.clientCache = CacheBuilder.newBuilder() + .recordStats() .expireAfterAccess(staleThresholdMs, MILLISECONDS) .maximumSize(clientConf.getMaxSize()) .removalListener( @@ -117,6 +121,8 @@ public void onRemoval( topologyAwareRead = conf.getBoolean( OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_KEY, OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_DEFAULT); + + cacheMetrics = CacheMetrics.create(clientCache, "xceiverClientCache"); } @VisibleForTesting @@ -278,6 +284,10 @@ public void close() { //closing is done through RemovalListener clientCache.invalidateAll(); clientCache.cleanUp(); + if (LOG.isDebugEnabled()) { + LOG.debug("XceiverClient cache stats: {}", clientCache.stats()); + } + cacheMetrics.unregister(); if (metrics != null) { metrics.unRegister(); From c55bc194b0486ee680b68d02a64405235317f6e3 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Mon, 26 Jun 2023 08:41:16 +0200 Subject: [PATCH 2/5] close XceiverClientManager in TestECBlockOutputStreamEntry --- .../io/TestECBlockOutputStreamEntry.java | 64 ++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/io/TestECBlockOutputStreamEntry.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/io/TestECBlockOutputStreamEntry.java index 4b87ef74cef1..804b3679a61b 100644 --- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/io/TestECBlockOutputStreamEntry.java +++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/io/TestECBlockOutputStreamEntry.java @@ -60,21 +60,22 @@ public class TestECBlockOutputStreamEntry { .setState(Pipeline.PipelineState.OPEN) .setNodes(nodes) .build(); - XceiverClientManager manager = - new XceiverClientManager(new OzoneConfiguration()); - HashSet clients = new HashSet<>(); - ECBlockOutputStreamEntry entry = new ECBlockOutputStreamEntry.Builder() - .setXceiverClientManager(manager) - .setPipeline(anECPipeline) - .build(); - for (int i = 0; i < nodes.size(); i++) { - clients.add( - manager.acquireClient( - entry.createSingleECBlockPipeline( - anECPipeline, nodes.get(i), i - ))); + try (XceiverClientManager manager = + new XceiverClientManager(new OzoneConfiguration())) { + HashSet clients = new HashSet<>(); + ECBlockOutputStreamEntry entry = new ECBlockOutputStreamEntry.Builder() + .setXceiverClientManager(manager) + .setPipeline(anECPipeline) + .build(); + for (int i = 0; i < nodes.size(); i++) { + clients.add( + manager.acquireClient( + entry.createSingleECBlockPipeline( + anECPipeline, nodes.get(i), i + ))); + } + assertEquals(5, clients.size()); } - assertEquals(5, clients.size()); } @Test @@ -97,23 +98,26 @@ public class TestECBlockOutputStreamEntry { .setState(Pipeline.PipelineState.OPEN) .setNodes(nodes) .build(); - XceiverClientManager manager = - new XceiverClientManager(new OzoneConfiguration()); - HashSet clients = new HashSet<>(); - ECBlockOutputStreamEntry entry = new ECBlockOutputStreamEntry.Builder() - .setXceiverClientManager(manager) - .setPipeline(anECPipeline) - .build(); - for (int i = 0; i < nodes.size(); i++) { - clients.add( - manager.acquireClient( - entry.createSingleECBlockPipeline( - anECPipeline, nodes.get(i), i - ))); + try (XceiverClientManager manager = + new XceiverClientManager(new OzoneConfiguration())) { + HashSet clients = new HashSet<>(); + ECBlockOutputStreamEntry entry = new ECBlockOutputStreamEntry.Builder() + .setXceiverClientManager(manager) + .setPipeline(anECPipeline) + .build(); + for (int i = 0; i < nodes.size(); i++) { + clients.add( + manager.acquireClient( + entry.createSingleECBlockPipeline( + anECPipeline, nodes.get(i), i + ))); + } + assertEquals(3, clients.size()); + assertEquals(1, + clients.stream().filter(c -> c.getRefcount() == 3).count()); + assertEquals(2, + clients.stream().filter(c -> c.getRefcount() == 1).count()); } - assertEquals(3, clients.size()); - assertEquals(1, clients.stream().filter(c -> c.getRefcount() == 3).count()); - assertEquals(2, clients.stream().filter(c -> c.getRefcount() == 1).count()); } private DatanodeDetails aNode(String ip, String hostName, int port) { From 081745dac7c33df621a4c8ff65b3496ecdf2b8cd Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Mon, 26 Jun 2023 08:42:15 +0200 Subject: [PATCH 3/5] use unique source name for CacheMetrics instances --- .../hadoop/hdds/scm/XceiverClientManager.java | 2 +- .../org/apache/hadoop/util/CacheMetrics.java | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java index ab9c0ef1d5da..8df6b2082bd6 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java @@ -122,7 +122,7 @@ public void onRemoval( OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_KEY, OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_DEFAULT); - cacheMetrics = CacheMetrics.create(clientCache, "xceiverClientCache"); + cacheMetrics = CacheMetrics.create(clientCache, this); } @VisibleForTesting diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java index 8f505f7604df..5f268d006df1 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java @@ -25,11 +25,13 @@ import org.apache.hadoop.metrics2.MetricsSource; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; +import org.apache.ratis.util.JavaUtils; /** * Reusable component that emits cache metrics for a particular cache. */ public final class CacheMetrics implements MetricsSource { + enum CacheMetricsInfo implements MetricsInfo { CacheName("Cache Metrics."), Size("Size of the cache."), @@ -62,16 +64,24 @@ public String description() { private final Cache cache; private final String name; + private final String sourceName; private CacheMetrics(Cache cache, String name) { this.cache = cache; this.name = name; + sourceName = NAME + ":" + name; + } + + public static CacheMetrics create(Cache cache, Object owner) { + final String name = JavaUtils.getClassSimpleName(owner.getClass()) + + "@" + Integer.toHexString(owner.hashCode()); + return create(cache, name); } public static CacheMetrics create(Cache cache, String name) { MetricsSystem ms = DefaultMetricsSystem.instance(); - return ms.register(NAME, "Cache Metrics", - new CacheMetrics(cache, name)); + CacheMetrics source = new CacheMetrics(cache, name); + return ms.register(source.sourceName, "Cache Metrics", source); } @Override @@ -98,6 +108,6 @@ public void getMetrics(MetricsCollector collector, boolean all) { public void unregister() { MetricsSystem ms = DefaultMetricsSystem.instance(); - ms.unregisterSource(NAME); + ms.unregisterSource(sourceName); } } From 9549140d03db5bce15e09e5681413616f87758d4 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Mon, 26 Jun 2023 09:03:49 +0200 Subject: [PATCH 4/5] remove duplicate constant in CacheMetrics --- .../src/main/java/org/apache/hadoop/util/CacheMetrics.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java index 5f268d006df1..e8a1788a991e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java @@ -60,8 +60,6 @@ public String description() { public static final String SOURCE_NAME = CacheMetrics.class.getSimpleName(); - public static final String NAME = CacheMetrics.class.getSimpleName(); - private final Cache cache; private final String name; private final String sourceName; @@ -69,7 +67,7 @@ public String description() { private CacheMetrics(Cache cache, String name) { this.cache = cache; this.name = name; - sourceName = NAME + ":" + name; + sourceName = SOURCE_NAME + ":" + name; } public static CacheMetrics create(Cache cache, Object owner) { From 0605709c1ca67411e3958e0d4b937aada5cad103 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Mon, 26 Jun 2023 10:52:55 +0200 Subject: [PATCH 5/5] Invalid character : in value part of property --- .../src/main/java/org/apache/hadoop/util/CacheMetrics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java index e8a1788a991e..d1c49402d0e6 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/CacheMetrics.java @@ -67,7 +67,7 @@ public String description() { private CacheMetrics(Cache cache, String name) { this.cache = cache; this.name = name; - sourceName = SOURCE_NAME + ":" + name; + sourceName = SOURCE_NAME + "-" + name; } public static CacheMetrics create(Cache cache, Object owner) {