You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Tejaskriya (via GitHub)" <gi...@apache.org> on 2024/12/03 08:53:39 UTC

[PR] HDDS-11711. Add metrics in SCM to print number of delete command sent and response received per datanode [ozone]

Tejaskriya opened a new pull request, #7522:
URL: https://github.com/apache/ozone/pull/7522

   
   ## What changes were proposed in this pull request?
   
   When there are many DNs, and if any DN is slow in processing. It is difficult to identify which DN is having issue unless we check the log deeply and can take long time. For fast debug in this PR, metrics are added in SCM per datanode to show how many delete command sent to datanode and how many response received or timedout. This will help in understand which DN is slow or having problem in much efficient way.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-11711
   
   ## How was this patch tested?
   
   Tested manually
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add metrics in SCM for number of delete commands sent and response received per datanode [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7522:
URL: https://github.com/apache/ozone/pull/7522#discussion_r1870823731


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -196,6 +227,119 @@ public long getNumBlockDeletionTransactionDataNodes() {
     return numBlockDeletionTransactionDataNodes.value();
   }
 
+  public Map<UUID, DatanodeCommandCounts> 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<UUID, DatanodeCommandCounts> e : numCommandsDatanode.entrySet()) {
+      recordBuilder = recordBuilder.endRecord().addRecord(SOURCE_NAME)
+          .add(new MetricsTag(Interns.info("datanode",
+              "Datanode host for deletion commands"), 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();
+  }
+
+  /**
+   *  Class contains metrics related to the ScmBlockDeletingService for each datanode.
+   */
+  public static final class DatanodeCommandCounts {
+    private long commandsSent;
+    private long commandsSuccess;
+    private long 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 = 0;
+      this.commandsSuccess = 0;
+      this.commandsFailure = 0;
+    }
+
+    public void incrCommandsSent(long delta) {
+      this.commandsSent += delta;
+    }
+
+    public void incrCommandsSuccess(long delta) {
+      this.commandsSuccess += delta;
+    }
+
+    public void incrCommandsFailure(long delta) {
+      this.commandsFailure += delta;
+    }
+
+    public long getCommandsSent() {
+      return commandsSent;
+    }
+
+    public long getCommandsSuccess() {
+      return commandsSuccess;
+    }
+
+    public long getCommandsFailure() {
+      return commandsFailure;
+    }
+
+    @Override
+    public String toString() {
+      return "Sent=" + commandsSent + ", Success=" + commandsSuccess + ", Failed=" + commandsFailure;
+    }
+  }
+
+  @VisibleForTesting
+  public int getNumCommandsDatanodeSent() {
+    int sent = 0;

Review Comment:
   Items being summed are `long`, this should be `long`, too.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -196,6 +227,119 @@ public long getNumBlockDeletionTransactionDataNodes() {
     return numBlockDeletionTransactionDataNodes.value();
   }
 
+  public Map<UUID, DatanodeCommandCounts> 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<UUID, DatanodeCommandCounts> e : numCommandsDatanode.entrySet()) {
+      recordBuilder = recordBuilder.endRecord().addRecord(SOURCE_NAME)
+          .add(new MetricsTag(Interns.info("datanode",
+              "Datanode host for deletion commands"), 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();
+  }
+
+  /**
+   *  Class contains metrics related to the ScmBlockDeletingService for each datanode.
+   */
+  public static final class DatanodeCommandCounts {
+    private long commandsSent;
+    private long commandsSuccess;
+    private long 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 = 0;
+      this.commandsSuccess = 0;
+      this.commandsFailure = 0;
+    }
+
+    public void incrCommandsSent(long delta) {
+      this.commandsSent += delta;
+    }
+
+    public void incrCommandsSuccess(long delta) {
+      this.commandsSuccess += delta;
+    }
+
+    public void incrCommandsFailure(long delta) {
+      this.commandsFailure += delta;
+    }
+
+    public long getCommandsSent() {
+      return commandsSent;
+    }
+
+    public long getCommandsSuccess() {
+      return commandsSuccess;
+    }
+
+    public long getCommandsFailure() {
+      return commandsFailure;
+    }
+
+    @Override
+    public String toString() {
+      return "Sent=" + commandsSent + ", Success=" + commandsSuccess + ", Failed=" + commandsFailure;
+    }
+  }
+
+  @VisibleForTesting

Review Comment:
   nit: I don't think we need this annotation.  It is usually applied to methods that do some internal operation, and are exposed only for tests.  These `getNum...()` methods exists only for tests, which is different.  They don't mutate internal state or do anything "dangerous" that should not be  in production code.  So I think having such safe public methods without adding the annotation is OK.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -196,6 +227,119 @@ public long getNumBlockDeletionTransactionDataNodes() {
     return numBlockDeletionTransactionDataNodes.value();
   }
 
+  public Map<UUID, DatanodeCommandCounts> 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<UUID, DatanodeCommandCounts> e : numCommandsDatanode.entrySet()) {
+      recordBuilder = recordBuilder.endRecord().addRecord(SOURCE_NAME)
+          .add(new MetricsTag(Interns.info("datanode",
+              "Datanode host for deletion commands"), 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();
+  }
+
+  /**
+   *  Class contains metrics related to the ScmBlockDeletingService for each datanode.
+   */
+  public static final class DatanodeCommandCounts {
+    private long commandsSent;
+    private long commandsSuccess;
+    private long 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 = 0;
+      this.commandsSuccess = 0;
+      this.commandsFailure = 0;
+    }
+
+    public void incrCommandsSent(long delta) {
+      this.commandsSent += delta;
+    }
+
+    public void incrCommandsSuccess(long delta) {
+      this.commandsSuccess += delta;
+    }
+
+    public void incrCommandsFailure(long delta) {
+      this.commandsFailure += delta;
+    }
+
+    public long getCommandsSent() {
+      return commandsSent;
+    }
+
+    public long getCommandsSuccess() {
+      return commandsSuccess;
+    }
+
+    public long getCommandsFailure() {
+      return commandsFailure;
+    }
+
+    @Override
+    public String toString() {
+      return "Sent=" + commandsSent + ", Success=" + commandsSuccess + ", Failed=" + commandsFailure;
+    }
+  }
+
+  @VisibleForTesting
+  public int getNumCommandsDatanodeSent() {
+    int sent = 0;
+    for (Map.Entry<UUID, DatanodeCommandCounts> e : numCommandsDatanode.entrySet()) {
+      sent += e.getValue().commandsSent;

Review Comment:
   Iterate `values()` instead of `entrySet()`.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -86,7 +100,11 @@ public final class ScmBlockDeletingServiceMetrics {
   @Metric(about = "The number of dataNodes of delete transactions.")
   private MutableGaugeLong numBlockDeletionTransactionDataNodes;
 
+  private Map<UUID, DatanodeCommandCounts> numCommandsDatanode;

Review Comment:
   Can be `final`.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -86,7 +100,11 @@ public final class ScmBlockDeletingServiceMetrics {
   @Metric(about = "The number of dataNodes of delete transactions.")
   private MutableGaugeLong numBlockDeletionTransactionDataNodes;
 
+  private Map<UUID, DatanodeCommandCounts> numCommandsDatanode;
+
   private ScmBlockDeletingServiceMetrics() {
+    this.registry = new MetricsRegistry(SOURCE_NAME);
+    numCommandsDatanode = new HashMap<>();

Review Comment:
   Please use `ConcurrentHashMap` for thread safety.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #7522:
URL: https://github.com/apache/ozone/pull/7522#issuecomment-2537897314

   @slfan1989 Although we can't have a direct comparison of the values for the different datanodes due to the reasons you have mentioned, we can still get useful information from the comparison between how many commands are being sent and how many are successful. If deletion is indeed stuck at the datanode level due to some issues with DN not responding/executing the commands, It can be identified using these metrics.
   Moreover, we can get a picture of how the deletion commands are being distributed against the different datanodes


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #7522:
URL: https://github.com/apache/ozone/pull/7522#issuecomment-2522185499

   thanks @adoroszlai , I have fixed it with making the method synchronized


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #7522:
URL: https://github.com/apache/ozone/pull/7522#discussion_r1883297788


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -196,6 +226,116 @@ public long getNumBlockDeletionTransactionDataNodes() {
     return numBlockDeletionTransactionDataNodes.value();
   }
 
+  public Map<UUID, DatanodeCommandCounts> 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<UUID, DatanodeCommandCounts> e : numCommandsDatanode.entrySet()) {
+      recordBuilder = recordBuilder.endRecord().addRecord(SOURCE_NAME)
+          .add(new MetricsTag(Interns.info("datanode",
+              "Datanode host for deletion commands"), 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();
+  }
+
+  /**
+   *  Class contains metrics related to the ScmBlockDeletingService for each datanode.
+   */
+  public static final class DatanodeCommandCounts {
+    private long commandsSent;
+    private long commandsSuccess;
+    private long 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 = 0;
+      this.commandsSuccess = 0;
+      this.commandsFailure = 0;
+    }
+
+    public void incrCommandsSent(long delta) {
+      this.commandsSent += delta;
+    }
+
+    public void incrCommandsSuccess(long delta) {
+      this.commandsSuccess += delta;
+    }
+
+    public void incrCommandsFailure(long delta) {
+      this.commandsFailure += delta;
+    }
+
+    public long getCommandsSent() {
+      return commandsSent;
+    }
+
+    public long getCommandsSuccess() {
+      return commandsSuccess;
+    }
+
+    public long getCommandsFailure() {
+      return commandsFailure;
+    }
+
+    @Override
+    public String toString() {
+      return "Sent=" + commandsSent + ", Success=" + commandsSuccess + ", Failed=" + commandsFailure;
+    }
+  }
+
+  public long getNumCommandsDatanodeSent() {
+    long sent = 0;
+    for (DatanodeCommandCounts v : numCommandsDatanode.values()) {
+      sent += v.commandsSent;
+    }
+    return sent;
+  }
+  public long getNumCommandsDatanodeSuccess() {
+    long sent = 0;
+    for (DatanodeCommandCounts v : numCommandsDatanode.values()) {
+      sent += v.commandsSuccess;
+    }
+    return sent;
+  }
+  public long getNumCommandsDatanodeFailed() {
+    long sent = 0;

Review Comment:
   nit: `long failCount = 0;`



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -196,6 +226,116 @@ public long getNumBlockDeletionTransactionDataNodes() {
     return numBlockDeletionTransactionDataNodes.value();
   }
 
+  public Map<UUID, DatanodeCommandCounts> 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<UUID, DatanodeCommandCounts> e : numCommandsDatanode.entrySet()) {
+      recordBuilder = recordBuilder.endRecord().addRecord(SOURCE_NAME)
+          .add(new MetricsTag(Interns.info("datanode",
+              "Datanode host for deletion commands"), 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();
+  }
+
+  /**
+   *  Class contains metrics related to the ScmBlockDeletingService for each datanode.
+   */
+  public static final class DatanodeCommandCounts {
+    private long commandsSent;
+    private long commandsSuccess;
+    private long 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 = 0;
+      this.commandsSuccess = 0;
+      this.commandsFailure = 0;
+    }
+
+    public void incrCommandsSent(long delta) {
+      this.commandsSent += delta;
+    }
+
+    public void incrCommandsSuccess(long delta) {
+      this.commandsSuccess += delta;
+    }
+
+    public void incrCommandsFailure(long delta) {
+      this.commandsFailure += delta;
+    }
+
+    public long getCommandsSent() {
+      return commandsSent;
+    }
+
+    public long getCommandsSuccess() {
+      return commandsSuccess;
+    }
+
+    public long getCommandsFailure() {
+      return commandsFailure;
+    }
+
+    @Override
+    public String toString() {
+      return "Sent=" + commandsSent + ", Success=" + commandsSuccess + ", Failed=" + commandsFailure;
+    }
+  }
+
+  public long getNumCommandsDatanodeSent() {
+    long sent = 0;
+    for (DatanodeCommandCounts v : numCommandsDatanode.values()) {
+      sent += v.commandsSent;
+    }
+    return sent;
+  }
+  public long getNumCommandsDatanodeSuccess() {
+    long sent = 0;

Review Comment:
   nit: `long successCount = 0;`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #7522:
URL: https://github.com/apache/ozone/pull/7522#issuecomment-2535469118

   @Tejaskriya I have a question: The number of blocks deleted by each DN when processing a delete command is inconsistent, which can be observed by enabling the DN’s debug logs. If there is no performance difference between DNs, the varying number of blocks to be deleted could still result in differences in deletion speed. So, how do we determine if there is a difference in deletion speed of a DN, based solely on whether the delete command succeeds or not?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #7522:
URL: https://github.com/apache/ozone/pull/7522#discussion_r1883536775


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -86,10 +99,14 @@ public final class ScmBlockDeletingServiceMetrics {
   @Metric(about = "The number of dataNodes of delete transactions.")
   private MutableGaugeLong numBlockDeletionTransactionDataNodes;
 
+  private final Map<UUID, DatanodeCommandCounts> numCommandsDatanode;
+
   private ScmBlockDeletingServiceMetrics() {
+    this.registry = new MetricsRegistry(SOURCE_NAME);

Review Comment:
   If we remove it we get this error:
   ```
   org.apache.hadoop.metrics2.MetricsException: Hybrid metrics: registry required.
   	at org.apache.hadoop.metrics2.lib.MetricsSourceBuilder.build(MetricsSourceBuilder.java:79)
   	at org.apache.hadoop.metrics2.impl.MetricsSystemImpl.register(MetricsSystemImpl.java:224)
   	at org.apache.hadoop.hdds.scm.block.ScmBlockDeletingServiceMetrics.create(ScmBlockDeletingServiceMetrics.java:109)
   	...
   ```
   So we require it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #7522:
URL: https://github.com/apache/ozone/pull/7522#discussion_r1881956817


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/ScmBlockDeletingServiceMetrics.java:
##########
@@ -86,10 +99,14 @@ public final class ScmBlockDeletingServiceMetrics {
   @Metric(about = "The number of dataNodes of delete transactions.")
   private MutableGaugeLong numBlockDeletionTransactionDataNodes;
 
+  private final Map<UUID, DatanodeCommandCounts> numCommandsDatanode;
+
   private ScmBlockDeletingServiceMetrics() {
+    this.registry = new MetricsRegistry(SOURCE_NAME);

Review Comment:
   verify if registry can be removed ... seems no usages here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl merged PR #7522:
URL: https://github.com/apache/ozone/pull/7522


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7522:
URL: https://github.com/apache/ozone/pull/7522#issuecomment-2520369515

   Thanks @Tejaskriya for updating the patch.
   
   `findbugs` [complains](https://github.com/Tejaskriya/ozone/actions/runs/12180200384/job/33974013839#step:8:15):
   
   ```
   M M LI: Incorrect lazy initialization of static field org.apache.hadoop.hdds.scm.block.ScmBlockDeletingServiceMetrics.instance in org.apache.hadoop.hdds.scm.block.ScmBlockDeletingServiceMetrics.create()  At ScmBlockDeletingServiceMetrics.java:[lines 110-112]
   ```
   
   but `create()` is not being changed in this PR, so this is a flaky check in findbugs.  (Not the first one we encounter.)  We still need to fix it, otherwise it will keep complaining on `master` if this is merged.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #7522:
URL: https://github.com/apache/ozone/pull/7522#issuecomment-2540853524

   @ashishkumar50 @sumitagrawl thank you for the reviews, I have addressed the comments


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add metrics in SCM to print number of delete command sent and response received per datanode [ozone]

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #7522:
URL: https://github.com/apache/ozone/pull/7522#issuecomment-2519428444

   @sumitagrawl @ashishkumar50 could you please help in reviewing this patch?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11711. Add SCM metrics for delete commands sent and response received per datanode [ozone]

Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #7522:
URL: https://github.com/apache/ozone/pull/7522#issuecomment-2520252982

   @adoroszlai thanks for the review, I have addressed all the comments


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org