This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new 7a2bd38538 HDDS-10063. NumKeys metric not decremented on FSO directory 
delete. (#5933)
7a2bd38538 is described below

commit 7a2bd3853830a0b3a9116fbf8062aa323e70f75f
Author: Ethan Rose <[email protected]>
AuthorDate: Mon Jan 8 02:33:41 2024 -0800

    HDDS-10063. NumKeys metric not decremented on FSO directory delete. (#5933)
---
 .../org/apache/hadoop/ozone/om/TestOmMetrics.java  | 116 +++++++++++++++++++++
 .../key/OMDirectoriesPurgeRequestWithFSO.java      |   6 +-
 2 files changed, 121 insertions(+), 1 deletion(-)

diff --git 
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
 
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
index 8ab1cf54a5..61913363b3 100644
--- 
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
+++ 
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
@@ -17,6 +17,9 @@
 package org.apache.hadoop.ozone.om;
 
 import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_ROOT;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
 import static 
org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.BUCKET;
 import static 
org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.VOLUME;
 import static org.apache.hadoop.ozone.security.acl.OzoneObj.StoreType.OZONE;
@@ -25,6 +28,7 @@ import static 
org.apache.hadoop.test.MetricsAsserts.getMetrics;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
@@ -35,6 +39,10 @@ import java.util.List;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
 import org.apache.hadoop.hdds.utils.IOUtils;
 import org.apache.hadoop.hdds.HddsConfigKeys;
 import org.apache.hadoop.hdds.client.BlockID;
@@ -51,10 +59,13 @@ import org.apache.hadoop.metrics2.MetricsRecordBuilder;
 import org.apache.hadoop.ozone.MiniOzoneCluster;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.OzoneConfigKeys;
+import org.apache.hadoop.ozone.OzoneConsts;
 import org.apache.hadoop.ozone.TestDataUtil;
 import org.apache.hadoop.ozone.client.ObjectStore;
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
 import org.apache.hadoop.ozone.om.helpers.OmBucketArgs;
 import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
 import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
@@ -66,11 +77,15 @@ import 
org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
 import org.apache.hadoop.ozone.security.acl.OzoneObj;
 import org.apache.hadoop.ozone.security.acl.OzoneObjInfo;
 import org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse;
+import org.apache.hadoop.test.MetricsAsserts;
+import org.apache.ozone.test.GenericTestUtils;
 import org.assertj.core.util.Lists;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
 import org.mockito.Mockito;
 
 /**
@@ -384,6 +399,107 @@ public class TestOmMetrics {
 
   }
 
+  @ParameterizedTest
+  @EnumSource(value = BucketLayout.class, names = {"FILE_SYSTEM_OPTIMIZED", 
"LEGACY"})
+  public void testDirectoryOps(BucketLayout bucketLayout) throws Exception {
+    clusterBuilder.setNumDatanodes(3);
+    conf.setBoolean(HddsConfigKeys.HDDS_SCM_SAFEMODE_ENABLED, true);
+    // Speed up background directory deletion for this test.
+    conf.setTimeDuration(OMConfigKeys.OZONE_DIR_DELETING_SERVICE_INTERVAL, 1, 
TimeUnit.SECONDS);
+    conf.set(OzoneConfigKeys.OZONE_CLIENT_FS_DEFAULT_BUCKET_LAYOUT, 
bucketLayout.name());
+    // For testing fs operations with legacy buckets.
+    conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+    startCluster();
+
+    // How long to wait for directory deleting service to clean up the files 
before aborting the test.
+    final int timeoutMillis =
+        
(int)conf.getTimeDuration(OMConfigKeys.OZONE_DIR_DELETING_SERVICE_INTERVAL, 0, 
TimeUnit.MILLISECONDS) * 3;
+    assertTrue(timeoutMillis > 0, "Failed to read directory deleting service 
interval. Retrieved " + timeoutMillis +
+        " milliseconds");
+
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+
+    // Cluster should be empty.
+    MetricsRecordBuilder omMetrics = getMetrics("OMMetrics");
+    assertCounter("NumKeys", 0L, omMetrics);
+    assertCounter("NumCreateDirectory", 0L, omMetrics);
+    // These key operations include directory operations.
+    assertCounter("NumKeyDeletes", 0L, omMetrics);
+    assertCounter("NumKeyRenames", 0L, omMetrics);
+
+    // Create bucket with 2 nested directories.
+    String rootPath = String.format("%s://%s/",
+        OzoneConsts.OZONE_OFS_URI_SCHEME, conf.get(OZONE_OM_ADDRESS_KEY));
+    conf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+    FileSystem fs = FileSystem.get(conf);
+    Path bucketPath = new Path(OZONE_ROOT, String.join(OZONE_URI_DELIMITER, 
volumeName, bucketName));
+    Path dirPath = new Path(bucketPath, String.join(OZONE_URI_DELIMITER, 
"dir1", "dir2"));
+    fs.mkdirs(dirPath);
+    assertEquals(bucketLayout,
+        
client.getObjectStore().getVolume(volumeName).getBucket(bucketName).getBucketLayout());
+    omMetrics = getMetrics("OMMetrics");
+    assertCounter("NumKeys", 2L, omMetrics);
+    // Only one directory create command is given, even though it created two 
directories.
+    assertCounter("NumCreateDirectory", 1L, omMetrics);
+    assertCounter("NumKeyDeletes", 0L, omMetrics);
+    assertCounter("NumKeyRenames", 0L, omMetrics);
+
+    // Add 2 files at different parts of the tree.
+    ContractTestUtils.touch(fs, new Path(dirPath, "file1"));
+    ContractTestUtils.touch(fs, new Path(dirPath.getParent(), "file2"));
+    omMetrics = getMetrics("OMMetrics");
+    assertCounter("NumKeys", 4L, omMetrics);
+    assertCounter("NumCreateDirectory", 1L, omMetrics);
+    assertCounter("NumKeyDeletes", 0L, omMetrics);
+    assertCounter("NumKeyRenames", 0L, omMetrics);
+
+    // Rename the child directory.
+    fs.rename(dirPath, new Path(dirPath.getParent(), "new-name"));
+    omMetrics = getMetrics("OMMetrics");
+    assertCounter("NumKeys", 4L, omMetrics);
+    assertCounter("NumCreateDirectory", 1L, omMetrics);
+    assertCounter("NumKeyDeletes", 0L, omMetrics);
+    long expectedRenames = 1;
+    if (bucketLayout == BucketLayout.LEGACY) {
+      // Legacy bucket must rename keys individually.
+      expectedRenames = 2;
+    }
+    assertCounter("NumKeyRenames", expectedRenames, omMetrics);
+
+    // Delete metric should be decremented by directory deleting service in 
the background.
+    fs.delete(dirPath.getParent(), true);
+    GenericTestUtils.waitFor(() -> {
+      long keyCount = MetricsAsserts.getLongCounter("NumKeys", 
getMetrics("OMMetrics"));
+      return keyCount == 0;
+    }, timeoutMillis / 5, timeoutMillis);
+    omMetrics = getMetrics("OMMetrics");
+    assertCounter("NumKeys", 0L, omMetrics);
+    // This is the number of times the create directory command was given, not 
the current number of directories.
+    assertCounter("NumCreateDirectory", 1L, omMetrics);
+    // Directory delete counts as key delete. One command was given so the 
metric is incremented once.
+    assertCounter("NumKeyDeletes", 1L, omMetrics);
+    assertCounter("NumKeyRenames", expectedRenames, omMetrics);
+
+    // Re-create the same tree as before, but this time delete the bucket 
recursively.
+    // All metrics should still be properly updated.
+    fs.mkdirs(dirPath);
+    ContractTestUtils.touch(fs, new Path(dirPath, "file1"));
+    ContractTestUtils.touch(fs, new Path(dirPath.getParent(), "file2"));
+    assertCounter("NumKeys", 4L, getMetrics("OMMetrics"));
+    fs.delete(bucketPath, true);
+    GenericTestUtils.waitFor(() -> {
+      long keyCount = MetricsAsserts.getLongCounter("NumKeys", 
getMetrics("OMMetrics"));
+      return keyCount == 0;
+    }, timeoutMillis / 5, timeoutMillis);
+    omMetrics = getMetrics("OMMetrics");
+    assertCounter("NumKeys", 0L, omMetrics);
+    assertCounter("NumCreateDirectory", 2L, omMetrics);
+    // One more keys delete request is given as part of the bucket delete to 
do a batch delete of its keys.
+    assertCounter("NumKeyDeletes", 2L, omMetrics);
+    assertCounter("NumKeyRenames", expectedRenames, omMetrics);
+  }
+
   @Test
   public void testSnapshotOps() throws Exception {
     // This tests needs enough dataNodes to allocate the blocks for the keys.
diff --git 
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
 
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
index 52bcf366c9..505b628730 100644
--- 
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
+++ 
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
@@ -24,6 +24,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.ozone.om.OMMetrics;
 import org.apache.ratis.server.protocol.TermIndex;
 import org.apache.hadoop.ozone.om.OMMetadataManager;
 import org.apache.hadoop.ozone.om.OzoneManager;
@@ -65,6 +66,8 @@ public class OMDirectoriesPurgeRequestWithFSO extends 
OMKeyRequest {
     Set<Pair<String, String>> lockSet = new HashSet<>();
     Map<Pair<String, String>, OmBucketInfo> volBucketInfoMap = new HashMap<>();
     OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+
+    OMMetrics omMetrics = ozoneManager.getMetrics();
     try {
       if (fromSnapshot != null) {
         fromSnapshotInfo = ozoneManager.getMetadataManager()
@@ -84,7 +87,7 @@ public class OMDirectoriesPurgeRequestWithFSO extends 
OMKeyRequest {
                 volumeName, bucketName);
             lockSet.add(volBucketPair);
           }
-          
+          omMetrics.decNumKeys();
           OmBucketInfo omBucketInfo = getBucketInfo(omMetadataManager,
               volumeName, bucketName);
           // bucketInfo can be null in case of delete volume or bucket
@@ -107,6 +110,7 @@ public class OMDirectoriesPurgeRequestWithFSO extends 
OMKeyRequest {
                 volumeName, bucketName);
             lockSet.add(volBucketPair);
           }
+          omMetrics.decNumKeys();
           OmBucketInfo omBucketInfo = getBucketInfo(omMetadataManager,
               volumeName, bucketName);
           // bucketInfo can be null in case of delete volume or bucket


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to