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]