[GitHub] [hadoop] bshashikant commented on a change in pull request #2595: HDFS-15619 Add snapshot related metrics

[GitBox <gi...@apache.org>]

bshashikant commented on a change in pull request #2595:
URL: https://github.com/apache/hadoop/pull/2595#discussion_r567939782



##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/metrics/NameNodeMetrics.java
##########
@@ -459,4 +472,23 @@ public void addEditLogTailInterval(long elapsed) {
       q.add(elapsed);
     }
   }
+  public void incrNumSnapshotDelete() {
+    numSnapshotTotalDeleteOp.incr();

Review comment:
       Its already an existing metric...Let's remove it.

##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
##########
@@ -519,6 +520,14 @@ public void deleteSnapshot(final INodesInPath iip, final String snapshotName,
         final List<XAttr> xattrs = Lists.newArrayListWithCapacity(1);
         xattrs.add(snapshotXAttr);
 
+        // Check if snapshot is already marked as deleted.
+        // If already marked deleted, metric for out of order snapshot deletion
+        // won't be incremented.
+        XAttr existingXAttr = FSDirXAttrOp.getXAttrByPrefixedName(
+            fsdir, INodesInPath.append(iip, snapshot.getRoot(),

Review comment:
       we can use snaphot.getRoot().isMarkedAsDeleted() here instead?

##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/metrics/NameNodeMetrics.java
##########
@@ -459,4 +472,23 @@ public void addEditLogTailInterval(long elapsed) {
       q.add(elapsed);
     }
   }
+  public void incrNumSnapshotDelete() {
+    numSnapshotTotalDeleteOp.incr();
+  }
+  public void incrOutOfOrderSnapshotDelete() {
+    numSnapshotOutOfOrderDeleteOp.incr();
+  }
+  public void incrInOrderSnapshotDelete() {
+    numSnapshotInOrderDeleteOp.incr();
+  }

Review comment:
       its better to move these 2 metrics to SnapshotMetrcics.




----------------------------------------------------------------
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.

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



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