You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ad...@apache.org on 2023/04/15 10:25:33 UTC
[ozone] branch master updated: HDDS-8242. Rename operation not working with FSO bucket destination (#4478)
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 c9424217cd HDDS-8242. Rename operation not working with FSO bucket destination (#4478)
c9424217cd is described below
commit c9424217cdf2de21612471425a37328d34ca105f
Author: Zita Dombi <50...@users.noreply.github.com>
AuthorDate: Sat Apr 15 12:25:27 2023 +0200
HDDS-8242. Rename operation not working with FSO bucket destination (#4478)
---
.../fs/ozone/TestRootedOzoneFileSystemWithFSO.java | 15 +++++++
.../client/rpc/TestOzoneRpcClientAbstract.java | 21 ++++++---
.../hadoop/ozone/om/TestObjectStoreWithFSO.java | 13 +++---
.../hadoop/ozone/om/request/OMClientRequest.java | 2 +-
.../ozone/om/request/key/OMKeyRenameRequest.java | 33 ++++++++++----
.../om/request/key/OMKeyRenameRequestWithFSO.java | 52 +++++++++++++++++++---
.../request/key/TestOMKeyRenameRequestWithFSO.java | 17 +++++--
7 files changed, 121 insertions(+), 32 deletions(-)
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java
index 7846a01842..1b78da7ce0 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java
@@ -137,6 +137,21 @@ public class TestRootedOzoneFileSystemWithFSO
Assert.assertFalse(getFs().rename(dir2SourcePath, newDestinPath));
}
+ @Test
+ public void testKeyRenameToBucketLevel() throws IOException {
+ final String dir = "dir1";
+ final String key = dir + "/key1";
+ final Path source = new Path(getBucketPath(), key);
+ getFs().mkdirs(source);
+ final Path dest = new Path(String.valueOf(getBucketPath()));
+ LOG.info("Will move {} to {}", source, dest);
+ getFs().rename(source, getBucketPath());
+ assertTrue("Key rename failed",
+ getFs().exists(new Path(getBucketPath(), "key1")));
+ // cleanup
+ getFs().delete(dest, true);
+ }
+
@Test
public void testRenameDir() throws Exception {
final String dir = "dir1";
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
index 78acd0094a..464eac7f64 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
@@ -2092,16 +2092,25 @@ public abstract class TestOzoneRpcClientAbstract {
volume.createBucket(bucketName);
OzoneBucket bucket = volume.getBucket(bucketName);
createTestKey(bucket, fromKeyName, value);
-
- // Rename to empty string should fail.
+ BucketLayout bucketLayout = bucket.getBucketLayout();
OMException oe = null;
String toKeyName = "";
- try {
+
+ if (!bucketLayout.isFileSystemOptimized()) {
+ // Rename to an empty string should fail only in non FSO buckets
+ try {
+ bucket.renameKey(fromKeyName, toKeyName);
+ } catch (OMException e) {
+ oe = e;
+ }
+ Assert.assertEquals(ResultCodes.INVALID_KEY_NAME, oe.getResult());
+ } else {
+ // Rename to an empty key in FSO should be okay, as we are handling the
+ // empty dest key on the server side and the source key name will be used
bucket.renameKey(fromKeyName, toKeyName);
- } catch (OMException e) {
- oe = e;
+ OzoneKey emptyRenameKey = bucket.getKey(fromKeyName);
+ Assert.assertEquals(fromKeyName, emptyRenameKey.getName());
}
- Assert.assertEquals(ResultCodes.INVALID_KEY_NAME, oe.getResult());
toKeyName = UUID.randomUUID().toString();
bucket.renameKey(fromKeyName, toKeyName);
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreWithFSO.java
index 52ce29bf1b..2da98434bb 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreWithFSO.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreWithFSO.java
@@ -623,15 +623,12 @@ public class TestObjectStoreWithFSO {
OzoneBucket bucket = volume.getBucket(bucketName);
createTestKey(bucket, fromKeyName, value);
- // Rename to empty string should fail.
+ // Rename to an empty string means that we are moving the key to the bucket
+ // level and the toKeyName will be the source key name
String toKeyName = "";
- try {
- bucket.renameKey(fromKeyName, toKeyName);
- fail("Rename to empty string should fail!");
- } catch (OMException ome) {
- Assert.assertEquals(OMException.ResultCodes.INVALID_KEY_NAME,
- ome.getResult());
- }
+ bucket.renameKey(fromKeyName, toKeyName);
+ OzoneKey emptyKeyRename = bucket.getKey(fromKeyName);
+ Assert.assertEquals(fromKeyName, emptyKeyRename.getName());
toKeyName = UUID.randomUUID().toString();
bucket.renameKey(fromKeyName, toKeyName);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
index 22cfc8037d..963eddc06c 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
@@ -534,7 +534,7 @@ public abstract class OMClientRequest implements RequestAuditor {
* ":", ".", "..", "//", "". If it has any of these characters throws
* OMException, else return the path.
*/
- private static String isValidKeyPath(String path) throws OMException {
+ public static String isValidKeyPath(String path) throws OMException {
boolean isValid = true;
// If keyName is empty string throw error.
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
index 2c934224bd..9ced78de0c 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
@@ -92,14 +92,8 @@ public class OMKeyRenameRequest extends OMKeyRequest {
KeyArgs renameKeyArgs = renameKeyRequest.getKeyArgs();
- String srcKey = renameKeyArgs.getKeyName();
- String dstKey = renameKeyRequest.getToKeyName();
- if (getBucketLayout().isFileSystemOptimized()) {
- srcKey = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
- srcKey, getBucketLayout());
- dstKey = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
- dstKey, getBucketLayout());
- }
+ String srcKey = extractSrcKey(renameKeyArgs);
+ String dstKey = extractDstKey(renameKeyRequest);
// Set modification time & srcKeyName.
KeyArgs.Builder newKeyArgs = renameKeyArgs.toBuilder()
@@ -284,4 +278,27 @@ public class OMKeyRenameRequest extends OMKeyRequest {
}
return req;
}
+
+ /**
+ * Returns the source key name.
+ *
+ * @param keyArgs
+ * @return source key name
+ * @throws OMException
+ */
+ protected String extractSrcKey(KeyArgs keyArgs) throws OMException {
+ return keyArgs.getKeyName();
+ }
+
+ /**
+ * Returns the destination key name.
+ *
+ * @param renameKeyRequest
+ * @return destination key name
+ * @throws OMException
+ */
+ protected String extractDstKey(RenameKeyRequest renameKeyRequest)
+ throws OMException {
+ return renameKeyRequest.getToKeyName();
+ }
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequestWithFSO.java
index 0220d1bf06..b9d28e5c3b 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequestWithFSO.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequestWithFSO.java
@@ -55,6 +55,7 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Map;
+import static org.apache.hadoop.ozone.OmUtils.normalizeKey;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
@@ -100,8 +101,8 @@ public class OMKeyRenameRequestWithFSO extends OMKeyRenameRequest {
OmKeyInfo fromKeyValue;
Result result;
try {
- if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
- throw new OMException("Key name is empty",
+ if (fromKeyName.length() == 0) {
+ throw new OMException("Source key name is empty",
OMException.ResultCodes.INVALID_KEY_NAME);
}
@@ -117,8 +118,14 @@ public class OMKeyRenameRequestWithFSO extends OMKeyRenameRequest {
IAccessAuthorizer.ACLType.DELETE);
// check Acl toKeyName
- checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
- IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ if (toKeyName.isEmpty()) {
+ // if the toKeyName is empty we are checking the ACLs of the bucket
+ checkBucketAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE);
+ } else {
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ }
acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
volumeName, bucketName);
@@ -254,7 +261,13 @@ public class OMKeyRenameRequestWithFSO extends OMKeyRenameRequest {
OmBucketInfo omBucketInfo = null;
final String dbFromKey = ommm.getOzonePathKey(volumeId, bucketId,
fromKeyValue.getParentObjectID(), fromKeyValue.getFileName());
- String toKeyFileName = OzoneFSUtils.getFileName(toKeyName);
+ String toKeyFileName;
+ if (toKeyName.isEmpty()) {
+ // if toKeyName is empty we use the source key name.
+ toKeyFileName = OzoneFSUtils.getFileName(fromKeyName);
+ } else {
+ toKeyFileName = OzoneFSUtils.getFileName(toKeyName);
+ }
OmKeyInfo fromKeyParent = null;
OMMetadataManager metadataMgr = ozoneManager.getMetadataManager();
Table<String, OmDirectoryInfo> dirTable = metadataMgr.getDirectoryTable();
@@ -355,4 +368,33 @@ public class OMKeyRenameRequestWithFSO extends OMKeyRenameRequest {
auditMap.put(OzoneConsts.DST_KEY, renameKeyRequest.getToKeyName());
return auditMap;
}
+
+ /**
+ * Returns the normalized and validated destination key name. It is handling
+ * the case when the toKeyName is empty (if we are renaming a key to bucket
+ * level, e.g. source is /vol1/buck1/dir1/key1 and dest is /vol1/buck1).
+ *
+ * @param request
+ * @return
+ * @throws OMException
+ */
+ @Override
+ protected String extractDstKey(RenameKeyRequest request) throws OMException {
+ String normalizedDstKey = normalizeKey(request.getToKeyName(), false);
+ return normalizedDstKey.isEmpty() ?
+ normalizedDstKey :
+ isValidKeyPath(normalizedDstKey);
+ }
+
+ /**
+ * Returns the validated and normalized source key name.
+ *
+ * @param keyArgs
+ * @return
+ * @throws OMException
+ */
+ @Override
+ protected String extractSrcKey(KeyArgs keyArgs) throws OMException {
+ return validateAndNormalizeKey(keyArgs.getKeyName());
+ }
}
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequestWithFSO.java
index d6d2f3388f..4289c2dcce 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequestWithFSO.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequestWithFSO.java
@@ -77,11 +77,19 @@ public class TestOMKeyRenameRequestWithFSO extends TestOMKeyRenameRequest {
@Override
@Test
public void testValidateAndUpdateCacheWithToKeyInvalid() throws Exception {
- String invalidToKeyName = "";
+ String invalidToKeyName = "invalid:";
Assert.assertThrows(
OMException.class, () -> doPreExecute(createRenameKeyRequest(
volumeName, bucketName, fromKeyName, invalidToKeyName))); }
+ @Test
+ public void testValidateAndUpdateCacheWithEmptyToKey() throws Exception {
+ String emptyToKeyName = "";
+ OMRequest omRequest = doPreExecute(createRenameKeyRequest(volumeName,
+ bucketName, fromKeyName, emptyToKeyName));
+ Assert.assertEquals(omRequest.getRenameKeyRequest().getToKeyName(), "");
+ }
+
@Override
@Test
public void testValidateAndUpdateCacheWithFromKeyInvalid() throws Exception {
@@ -133,10 +141,11 @@ public class TestOMKeyRenameRequestWithFSO extends TestOMKeyRenameRequest {
}
private OMRequest doPreExecute(OMRequest originalOmRequest) throws Exception {
- OMKeyRenameRequest omKeyRenameRequest =
- new OMKeyRenameRequest(originalOmRequest, getBucketLayout());
+ OMKeyRenameRequestWithFSO omKeyRenameRequestWithFSO =
+ new OMKeyRenameRequestWithFSO(originalOmRequest, getBucketLayout());
- OMRequest modifiedOmRequest = omKeyRenameRequest.preExecute(ozoneManager);
+ OMRequest modifiedOmRequest
+ = omKeyRenameRequestWithFSO.preExecute(ozoneManager);
// Will not be equal, as UserInfo will be set and modification time is
// set in KeyArgs.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org