From f5f409837326de25efa83286b31b99b670a9071d Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Tue, 3 Dec 2024 14:22:03 +0530 Subject: [PATCH 01/10] HDDS-11711. Add metrics in SCM to print number of delete command sent and response received per datanode --- .../hdds/scm/block/DeletedBlockLogImpl.java | 2 + .../scm/block/SCMBlockDeletingService.java | 1 + .../block/ScmBlockDeletingServiceMetrics.java | 121 +++++++++++++++++- hadoop-ozone/dist/src/main/compose/ozone/.env | 1 + .../commandhandler/TestBlockDeletion.java | 3 + 5 files changed, 127 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java index 45d6a0249388..d895699b94bb 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java @@ -445,8 +445,10 @@ public void onMessage( getSCMDeletedBlockTransactionStatusManager() .commitTransactions(ackProto.getResultsList(), dnId); metrics.incrBlockDeletionCommandSuccess(); + metrics.incrDNCommandsSuccess(dnId, 1); } else if (status == CommandStatus.Status.FAILED) { metrics.incrBlockDeletionCommandFailure(); + metrics.incrDNCommandsFailure(dnId, 1); } else { LOG.debug("Delete Block Command {} is not executed on the Datanode" + " {}.", commandStatus.getCmdId(), dnId); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java index e6fc45cb5eee..782ef05c4cb4 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java @@ -192,6 +192,7 @@ public EmptyTaskResult call() throws Exception { new CommandForDatanode<>(dnId, command)); metrics.incrBlockDeletionCommandSent(); metrics.incrBlockDeletionTransactionSent(dnTXs.size()); + metrics.incrDNCommandsSent(dnId, 1); if (LOG.isDebugEnabled()) { LOG.debug( "Added delete block command for datanode {} in the queue," diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index 6637bd183293..9e4045bc341a 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -19,23 +19,36 @@ package org.apache.hadoop.hdds.scm.block; +import org.apache.hadoop.metrics2.MetricsInfo; +import org.apache.hadoop.metrics2.MetricsCollector; +import org.apache.hadoop.metrics2.MetricsRecordBuilder; +import org.apache.hadoop.metrics2.MetricsSource; import org.apache.hadoop.metrics2.MetricsSystem; +import org.apache.hadoop.metrics2.MetricsTag; import org.apache.hadoop.metrics2.annotation.Metric; import org.apache.hadoop.metrics2.annotation.Metrics; import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; import org.apache.hadoop.metrics2.lib.MutableCounterLong; import org.apache.hadoop.metrics2.lib.MutableGaugeLong; +import org.apache.hadoop.metrics2.lib.MetricsRegistry; +import org.apache.hadoop.metrics2.lib.Interns; + +import java.util.Map; +import java.util.UUID; +import java.util.HashMap; + /** * Metrics related to Block Deleting Service running in SCM. */ @Metrics(name = "ScmBlockDeletingService Metrics", about = "Metrics related to " + "background block deleting service in SCM", context = "SCM") -public final class ScmBlockDeletingServiceMetrics { +public final class ScmBlockDeletingServiceMetrics implements MetricsSource { private static ScmBlockDeletingServiceMetrics instance; public static final String SOURCE_NAME = SCMBlockDeletingService.class.getSimpleName(); + private final MetricsRegistry registry; /** * Given all commands are finished and no new coming deletes from OM. @@ -86,7 +99,11 @@ public final class ScmBlockDeletingServiceMetrics { @Metric(about = "The number of dataNodes of delete transactions.") private MutableGaugeLong numBlockDeletionTransactionDataNodes; + private Map numCommandsDatanode; + private ScmBlockDeletingServiceMetrics() { + this.registry = new MetricsRegistry(SOURCE_NAME); + numCommandsDatanode = new HashMap<>(); } public static ScmBlockDeletingServiceMetrics create() { @@ -196,6 +213,108 @@ public long getNumBlockDeletionTransactionDataNodes() { return numBlockDeletionTransactionDataNodes.value(); } + public Map getNumCommandsDatanode() { + return numCommandsDatanode; + } + + @Override + public void getMetrics(MetricsCollector metricsCollector, boolean all) { + MetricsRecordBuilder builder = metricsCollector.addRecord(SOURCE_NAME); + numBlockDeletionCommandSent.snapshot(builder, all); + numBlockDeletionCommandSuccess.snapshot(builder, all); + numBlockDeletionCommandFailure.snapshot(builder, all); + numBlockDeletionTransactionSent.snapshot(builder, all); + numBlockDeletionTransactionSuccess.snapshot(builder, all); + numBlockDeletionTransactionFailure.snapshot(builder, all); + numBlockDeletionTransactionCompleted.snapshot(builder, all); + numBlockDeletionTransactionCreated.snapshot(builder, all); + numSkippedTransactions.snapshot(builder, all); + numProcessedTransactions.snapshot(builder, all); + numBlockDeletionTransactionDataNodes.snapshot(builder, all); + + MetricsRecordBuilder recordBuilder = builder; + for (Map.Entry e : numCommandsDatanode.entrySet()) { + recordBuilder = recordBuilder.endRecord().addRecord(SOURCE_NAME) + .add(new MetricsTag(Interns.info("datanode", + "Commands sent/received for the datanode"), e.getKey().toString())) + .addGauge(DatanodeCommandCounts.COMMANDS_SENT_TO_DN, + e.getValue().getCommandsSent()) + .addGauge(DatanodeCommandCounts.COMMANDS_SUCCESSFUL_EXECUTION_BY_DN, + e.getValue().getCommandsSuccess()) + .addGauge(DatanodeCommandCounts.COMMANDS_FAILED_EXECUTION_BY_DN, + e.getValue().getCommandsFailure()); + } + recordBuilder.endRecord(); + } + + @Metrics(name = "DatanodeCommandCounts", about = "Metrics for deletion commands per Datanode", context = "datanode") + public static final class DatanodeCommandCounts { + @Metric(about = "The number of delete commands sent to a DN.") + private MutableGaugeLong commandsSent; + @Metric(about = "The number of delete commands successful for a DN.") + private MutableGaugeLong commandsSuccess; + @Metric(about = "The number of delete commands failed for a DN.") + private MutableGaugeLong commandsFailure; + + private static final MetricsInfo COMMANDS_SENT_TO_DN = Interns.info( + "CommandsSent", + "Number of commands sent from SCM to the datanode for deletion"); + private static final MetricsInfo COMMANDS_SUCCESSFUL_EXECUTION_BY_DN = Interns.info( + "CommandsSuccess", + "Number of commands sent from SCM to the datanode for deletion for which execution succeeded."); + private static final MetricsInfo COMMANDS_FAILED_EXECUTION_BY_DN = Interns.info( + "CommandsFailed", + "Number of commands sent from SCM to the datanode for deletion for which execution failed."); + + public DatanodeCommandCounts() { + this.commandsSent.set(0); + this.commandsSuccess.set(0); + this.commandsFailure.set(0); + } + + public void incrCommandsSent(long delta) { + this.commandsSent.incr(delta); + } + + public void incrCommandsSuccess(long delta) { + this.commandsSuccess.incr(delta); + } + + public void incrCommandsFailure(long delta) { + this.commandsFailure.incr(delta); + } + + public long getCommandsSent() { + return commandsSent.value(); + } + + public long getCommandsSuccess() { + return commandsSuccess.value(); + } + + public long getCommandsFailure() { + return commandsFailure.value(); + } + + @Override + public String toString() { + return "Sent=" + commandsSent + ", Success=" + commandsSuccess + ", Failed=" + commandsFailure; + } + } + + public void incrDNCommandsSent(UUID id, long delta) { + numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) + .incrCommandsSent(delta); + } + public void incrDNCommandsSuccess(UUID id, long delta) { + numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) + .incrCommandsSuccess(delta); + } + public void incrDNCommandsFailure(UUID id, long delta) { + numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) + .incrCommandsFailure(delta); + } + @Override public String toString() { StringBuffer buffer = new StringBuffer(); diff --git a/hadoop-ozone/dist/src/main/compose/ozone/.env b/hadoop-ozone/dist/src/main/compose/ozone/.env index 6507664fad7f..4cc42c1311f1 100644 --- a/hadoop-ozone/dist/src/main/compose/ozone/.env +++ b/hadoop-ozone/dist/src/main/compose/ozone/.env @@ -16,6 +16,7 @@ HDDS_VERSION=${hdds.version} HADOOP_IMAGE=apache/hadoop +COMPOSE_FILE=docker-compose.yaml:monitoring.yaml OZONE_RUNNER_VERSION=${docker.ozone-runner.version} OZONE_RUNNER_IMAGE=apache/ozone-runner OZONE_OPTS= diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java index 719715ac8b3d..6a6158a2f8b6 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java @@ -317,6 +317,9 @@ public void testBlockDeletion(ReplicationConfig repConfig) throws Exception { assertEquals(metrics.getNumBlockDeletionTransactionCreated(), metrics.getNumBlockDeletionTransactionCompleted()); + LOG.info("tej1 : "+ " " + metrics.getNumBlockDeletionCommandSent() +" " + metrics.getNumBlockDeletionCommandSuccess() + + " " + metrics.getBNumBlockDeletionCommandFailure()); + LOG.info("tej2 : "+ " " + metrics.getNumCommandsDatanode()); assertThat(metrics.getNumBlockDeletionCommandSent()) .isGreaterThanOrEqualTo(metrics.getNumBlockDeletionCommandSuccess() + metrics.getBNumBlockDeletionCommandFailure()); From 2092fbdf7f3ceb42aa20a859c1dc5ec4fcd79819 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Wed, 4 Dec 2024 13:13:10 +0530 Subject: [PATCH 02/10] Remove unnecessary annotations --- .../block/ScmBlockDeletingServiceMetrics.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index 9e4045bc341a..138fca49c1d9 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -236,7 +236,7 @@ public void getMetrics(MetricsCollector metricsCollector, boolean all) { for (Map.Entry e : numCommandsDatanode.entrySet()) { recordBuilder = recordBuilder.endRecord().addRecord(SOURCE_NAME) .add(new MetricsTag(Interns.info("datanode", - "Commands sent/received for the datanode"), e.getKey().toString())) + "Datanode host for deletion commands"), e.getKey().toString())) .addGauge(DatanodeCommandCounts.COMMANDS_SENT_TO_DN, e.getValue().getCommandsSent()) .addGauge(DatanodeCommandCounts.COMMANDS_SUCCESSFUL_EXECUTION_BY_DN, @@ -247,14 +247,10 @@ public void getMetrics(MetricsCollector metricsCollector, boolean all) { recordBuilder.endRecord(); } - @Metrics(name = "DatanodeCommandCounts", about = "Metrics for deletion commands per Datanode", context = "datanode") public static final class DatanodeCommandCounts { - @Metric(about = "The number of delete commands sent to a DN.") - private MutableGaugeLong commandsSent; - @Metric(about = "The number of delete commands successful for a DN.") - private MutableGaugeLong commandsSuccess; - @Metric(about = "The number of delete commands failed for a DN.") - private MutableGaugeLong commandsFailure; + private long commandsSent; + private long commandsSuccess; + private long commandsFailure; private static final MetricsInfo COMMANDS_SENT_TO_DN = Interns.info( "CommandsSent", @@ -267,33 +263,33 @@ public static final class DatanodeCommandCounts { "Number of commands sent from SCM to the datanode for deletion for which execution failed."); public DatanodeCommandCounts() { - this.commandsSent.set(0); - this.commandsSuccess.set(0); - this.commandsFailure.set(0); + this.commandsSent = 0; + this.commandsSuccess = 0; + this.commandsFailure = 0; } public void incrCommandsSent(long delta) { - this.commandsSent.incr(delta); + this.commandsSent += delta; } public void incrCommandsSuccess(long delta) { - this.commandsSuccess.incr(delta); + this.commandsSuccess += delta; } public void incrCommandsFailure(long delta) { - this.commandsFailure.incr(delta); + this.commandsFailure += delta; } public long getCommandsSent() { - return commandsSent.value(); + return commandsSent; } public long getCommandsSuccess() { - return commandsSuccess.value(); + return commandsSuccess; } public long getCommandsFailure() { - return commandsFailure.value(); + return commandsFailure; } @Override @@ -333,7 +329,9 @@ public String toString() { .append("numBlockDeletionTransactionSuccess = " + numBlockDeletionTransactionSuccess.value()).append("\t") .append("numBlockDeletionTransactionFailure = " - + numBlockDeletionTransactionFailure.value()); + + numBlockDeletionTransactionFailure.value()).append("\t") + .append("numDeletionCommandsPerDatanode = " + + numCommandsDatanode); return buffer.toString(); } } From af41d82a7462af3061b75f0b85f18384cdb803a3 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Wed, 4 Dec 2024 13:26:37 +0530 Subject: [PATCH 03/10] Add assert for new metrics in tests --- .../block/ScmBlockDeletingServiceMetrics.java | 26 +++++++++++++++++++ .../commandhandler/TestBlockDeletion.java | 6 ++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index 138fca49c1d9..1aa8b2c58a05 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hdds.scm.block; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.metrics2.MetricsInfo; import org.apache.hadoop.metrics2.MetricsCollector; import org.apache.hadoop.metrics2.MetricsRecordBuilder; @@ -217,6 +218,31 @@ public Map getNumCommandsDatanode() { return numCommandsDatanode; } + @VisibleForTesting + public int getNumCommandsDatanodeSent() { + int sent = 0; + for(Map.Entry e : numCommandsDatanode.entrySet()) { + sent += e.getValue().commandsSent; + } + return sent; + } + @VisibleForTesting + public int getNumCommandsDatanodeSuccess() { + int sent = 0; + for(Map.Entry e : numCommandsDatanode.entrySet()) { + sent += e.getValue().commandsSuccess; + } + return sent; + } + @VisibleForTesting + public int getNumCommandsDatanodeFailed() { + int sent = 0; + for(Map.Entry e : numCommandsDatanode.entrySet()) { + sent += e.getValue().commandsFailure; + } + return sent; + } + @Override public void getMetrics(MetricsCollector metricsCollector, boolean all) { MetricsRecordBuilder builder = metricsCollector.addRecord(SOURCE_NAME); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java index 6a6158a2f8b6..7c65a473009e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java @@ -317,9 +317,9 @@ public void testBlockDeletion(ReplicationConfig repConfig) throws Exception { assertEquals(metrics.getNumBlockDeletionTransactionCreated(), metrics.getNumBlockDeletionTransactionCompleted()); - LOG.info("tej1 : "+ " " + metrics.getNumBlockDeletionCommandSent() +" " + metrics.getNumBlockDeletionCommandSuccess() - + " " + metrics.getBNumBlockDeletionCommandFailure()); - LOG.info("tej2 : "+ " " + metrics.getNumCommandsDatanode()); + assertEquals(metrics.getNumBlockDeletionCommandSent(), metrics.getNumCommandsDatanodeSent()); + assertEquals(metrics.getNumBlockDeletionCommandSuccess(), metrics.getNumCommandsDatanodeSuccess()); + assertEquals(metrics.getBNumBlockDeletionCommandFailure(), metrics.getNumCommandsDatanodeFailed()); assertThat(metrics.getNumBlockDeletionCommandSent()) .isGreaterThanOrEqualTo(metrics.getNumBlockDeletionCommandSuccess() + metrics.getBNumBlockDeletionCommandFailure()); From 48646ac2d6494869d23c28006c49a897e40f667c Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Wed, 4 Dec 2024 13:28:08 +0530 Subject: [PATCH 04/10] Remove env change in docker file --- hadoop-ozone/dist/src/main/compose/ozone/.env | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/dist/src/main/compose/ozone/.env b/hadoop-ozone/dist/src/main/compose/ozone/.env index 4cc42c1311f1..6507664fad7f 100644 --- a/hadoop-ozone/dist/src/main/compose/ozone/.env +++ b/hadoop-ozone/dist/src/main/compose/ozone/.env @@ -16,7 +16,6 @@ HDDS_VERSION=${hdds.version} HADOOP_IMAGE=apache/hadoop -COMPOSE_FILE=docker-compose.yaml:monitoring.yaml OZONE_RUNNER_VERSION=${docker.ozone-runner.version} OZONE_RUNNER_IMAGE=apache/ozone-runner OZONE_OPTS= From bc68a5fe0f6ad39f49bfd339f27a84c0b4c7a10e Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Wed, 4 Dec 2024 13:30:36 +0530 Subject: [PATCH 05/10] rearrange code in metrics class --- .../block/ScmBlockDeletingServiceMetrics.java | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index 1aa8b2c58a05..a50d075f1c29 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -170,6 +170,19 @@ public void setNumBlockDeletionTransactionDataNodes(long dataNodes) { this.numBlockDeletionTransactionDataNodes.set(dataNodes); } + public void incrDNCommandsSent(UUID id, long delta) { + numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) + .incrCommandsSent(delta); + } + public void incrDNCommandsSuccess(UUID id, long delta) { + numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) + .incrCommandsSuccess(delta); + } + public void incrDNCommandsFailure(UUID id, long delta) { + numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) + .incrCommandsFailure(delta); + } + public long getNumBlockDeletionCommandSent() { return numBlockDeletionCommandSent.value(); } @@ -218,31 +231,6 @@ public Map getNumCommandsDatanode() { return numCommandsDatanode; } - @VisibleForTesting - public int getNumCommandsDatanodeSent() { - int sent = 0; - for(Map.Entry e : numCommandsDatanode.entrySet()) { - sent += e.getValue().commandsSent; - } - return sent; - } - @VisibleForTesting - public int getNumCommandsDatanodeSuccess() { - int sent = 0; - for(Map.Entry e : numCommandsDatanode.entrySet()) { - sent += e.getValue().commandsSuccess; - } - return sent; - } - @VisibleForTesting - public int getNumCommandsDatanodeFailed() { - int sent = 0; - for(Map.Entry e : numCommandsDatanode.entrySet()) { - sent += e.getValue().commandsFailure; - } - return sent; - } - @Override public void getMetrics(MetricsCollector metricsCollector, boolean all) { MetricsRecordBuilder builder = metricsCollector.addRecord(SOURCE_NAME); @@ -324,17 +312,29 @@ public String toString() { } } - public void incrDNCommandsSent(UUID id, long delta) { - numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) - .incrCommandsSent(delta); + @VisibleForTesting + public int getNumCommandsDatanodeSent() { + int sent = 0; + for(Map.Entry e : numCommandsDatanode.entrySet()) { + sent += e.getValue().commandsSent; + } + return sent; } - public void incrDNCommandsSuccess(UUID id, long delta) { - numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) - .incrCommandsSuccess(delta); + @VisibleForTesting + public int getNumCommandsDatanodeSuccess() { + int sent = 0; + for(Map.Entry e : numCommandsDatanode.entrySet()) { + sent += e.getValue().commandsSuccess; + } + return sent; } - public void incrDNCommandsFailure(UUID id, long delta) { - numCommandsDatanode.computeIfAbsent(id, k -> new DatanodeCommandCounts()) - .incrCommandsFailure(delta); + @VisibleForTesting + public int getNumCommandsDatanodeFailed() { + int sent = 0; + for(Map.Entry e : numCommandsDatanode.entrySet()) { + sent += e.getValue().commandsFailure; + } + return sent; } @Override From 941c4f6caa0b67e08fc61e01e1af97f8d2dc3da3 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Wed, 4 Dec 2024 18:50:07 +0530 Subject: [PATCH 06/10] checkstyle fix --- .../hdds/scm/block/ScmBlockDeletingServiceMetrics.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index a50d075f1c29..93a43719e248 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -261,6 +261,9 @@ public void getMetrics(MetricsCollector metricsCollector, boolean all) { recordBuilder.endRecord(); } + /** + * Class contains metrics related to the ScmBlockDeletingService for each datanode. + */ public static final class DatanodeCommandCounts { private long commandsSent; private long commandsSuccess; @@ -315,7 +318,7 @@ public String toString() { @VisibleForTesting public int getNumCommandsDatanodeSent() { int sent = 0; - for(Map.Entry e : numCommandsDatanode.entrySet()) { + for (Map.Entry e : numCommandsDatanode.entrySet()) { sent += e.getValue().commandsSent; } return sent; @@ -323,7 +326,7 @@ public int getNumCommandsDatanodeSent() { @VisibleForTesting public int getNumCommandsDatanodeSuccess() { int sent = 0; - for(Map.Entry e : numCommandsDatanode.entrySet()) { + for (Map.Entry e : numCommandsDatanode.entrySet()) { sent += e.getValue().commandsSuccess; } return sent; @@ -331,7 +334,7 @@ public int getNumCommandsDatanodeSuccess() { @VisibleForTesting public int getNumCommandsDatanodeFailed() { int sent = 0; - for(Map.Entry e : numCommandsDatanode.entrySet()) { + for (Map.Entry e : numCommandsDatanode.entrySet()) { sent += e.getValue().commandsFailure; } return sent; From 12a971a22a7f990248a7fb9e58f2f08c4227e707 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Thu, 5 Dec 2024 18:23:49 +0530 Subject: [PATCH 07/10] Use concurrentHashMap and remove unnecessary test annotations --- .../block/ScmBlockDeletingServiceMetrics.java | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index 93a43719e248..b59a44b29fce 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -19,7 +19,6 @@ package org.apache.hadoop.hdds.scm.block; -import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.metrics2.MetricsInfo; import org.apache.hadoop.metrics2.MetricsCollector; import org.apache.hadoop.metrics2.MetricsRecordBuilder; @@ -37,7 +36,7 @@ import java.util.Map; import java.util.UUID; -import java.util.HashMap; +import java.util.concurrent.ConcurrentHashMap; /** * Metrics related to Block Deleting Service running in SCM. @@ -104,7 +103,7 @@ public final class ScmBlockDeletingServiceMetrics implements MetricsSource { private ScmBlockDeletingServiceMetrics() { this.registry = new MetricsRegistry(SOURCE_NAME); - numCommandsDatanode = new HashMap<>(); + numCommandsDatanode = new ConcurrentHashMap<>(); } public static ScmBlockDeletingServiceMetrics create() { @@ -315,27 +314,24 @@ public String toString() { } } - @VisibleForTesting - public int getNumCommandsDatanodeSent() { - int sent = 0; - for (Map.Entry e : numCommandsDatanode.entrySet()) { - sent += e.getValue().commandsSent; + public long getNumCommandsDatanodeSent() { + long sent = 0; + for (DatanodeCommandCounts v : numCommandsDatanode.values()) { + sent += v.commandsSent; } return sent; } - @VisibleForTesting - public int getNumCommandsDatanodeSuccess() { - int sent = 0; - for (Map.Entry e : numCommandsDatanode.entrySet()) { - sent += e.getValue().commandsSuccess; + public long getNumCommandsDatanodeSuccess() { + long sent = 0; + for (DatanodeCommandCounts v : numCommandsDatanode.values()) { + sent += v.commandsSuccess; } return sent; } - @VisibleForTesting - public int getNumCommandsDatanodeFailed() { - int sent = 0; - for (Map.Entry e : numCommandsDatanode.entrySet()) { - sent += e.getValue().commandsFailure; + public long getNumCommandsDatanodeFailed() { + long sent = 0; + for (DatanodeCommandCounts v : numCommandsDatanode.values()) { + sent += v.commandsFailure; } return sent; } From ef499e1e0f8de4ff52fdc398349afe88f93ae1ba Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Thu, 5 Dec 2024 18:26:52 +0530 Subject: [PATCH 08/10] final variable --- .../hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index b59a44b29fce..1fa397232e79 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -99,7 +99,7 @@ public final class ScmBlockDeletingServiceMetrics implements MetricsSource { @Metric(about = "The number of dataNodes of delete transactions.") private MutableGaugeLong numBlockDeletionTransactionDataNodes; - private Map numCommandsDatanode; + private final Map numCommandsDatanode; private ScmBlockDeletingServiceMetrics() { this.registry = new MetricsRegistry(SOURCE_NAME); From 72518d1907374ab2c7437e30661a04e8d15a5dd8 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Fri, 6 Dec 2024 11:09:36 +0530 Subject: [PATCH 09/10] synchronized initialization of static instance --- .../hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index 1fa397232e79..5b2f3cbc9011 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -106,7 +106,7 @@ private ScmBlockDeletingServiceMetrics() { numCommandsDatanode = new ConcurrentHashMap<>(); } - public static ScmBlockDeletingServiceMetrics create() { + public static synchronized ScmBlockDeletingServiceMetrics create() { if (instance == null) { MetricsSystem ms = DefaultMetricsSystem.instance(); instance = ms.register(SOURCE_NAME, "SCMBlockDeletingService", From 76a5829b97a5ddac138857dd2eab1ef7a4c1221b Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Fri, 13 Dec 2024 13:51:12 +0530 Subject: [PATCH 10/10] Change local variable names --- .../block/ScmBlockDeletingServiceMetrics.java | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java index 5b2f3cbc9011..d4dd7933ea76 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java @@ -99,11 +99,10 @@ public final class ScmBlockDeletingServiceMetrics implements MetricsSource { @Metric(about = "The number of dataNodes of delete transactions.") private MutableGaugeLong numBlockDeletionTransactionDataNodes; - private final Map numCommandsDatanode; + private final Map numCommandsDatanode = new ConcurrentHashMap<>(); private ScmBlockDeletingServiceMetrics() { this.registry = new MetricsRegistry(SOURCE_NAME); - numCommandsDatanode = new ConcurrentHashMap<>(); } public static synchronized ScmBlockDeletingServiceMetrics create() { @@ -226,10 +225,6 @@ public long getNumBlockDeletionTransactionDataNodes() { return numBlockDeletionTransactionDataNodes.value(); } - public Map getNumCommandsDatanode() { - return numCommandsDatanode; - } - @Override public void getMetrics(MetricsCollector metricsCollector, boolean all) { MetricsRecordBuilder builder = metricsCollector.addRecord(SOURCE_NAME); @@ -322,18 +317,18 @@ public long getNumCommandsDatanodeSent() { return sent; } public long getNumCommandsDatanodeSuccess() { - long sent = 0; + long successCount = 0; for (DatanodeCommandCounts v : numCommandsDatanode.values()) { - sent += v.commandsSuccess; + successCount += v.commandsSuccess; } - return sent; + return successCount; } public long getNumCommandsDatanodeFailed() { - long sent = 0; + long failCount = 0; for (DatanodeCommandCounts v : numCommandsDatanode.values()) { - sent += v.commandsFailure; + failCount += v.commandsFailure; } - return sent; + return failCount; } @Override