You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/02/25 16:46:10 UTC
[GitHub] [ozone] sadanand48 opened a new pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
sadanand48 opened a new pull request #1965:
URL: https://github.com/apache/ozone/pull/1965
## What changes were proposed in this pull request?
Add atomic rename and delete api's to BasicRootedOzoneFileSystem (OFS).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4490
## How was this patch tested?
Unit tests. (Same as TestRootedOzoneFileSystem) but with FSO configs. Will be adding more tests.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r585208740
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1830,14 +1837,23 @@ private OzoneFileStatus getOzoneFileStatus(String volumeName,
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
volumeName, bucketName, keyName);
+ // if the queried key is already a directory key (For eg: Keyname= a/b/c/)
+ if (fileKeyBytes.endsWith(OZONE_URI_DELIMITER)){
Review comment:
Should we need this additional check for the trailing slash? Will the below line fetch the given directory a/b/c/ or file a/b/c key from KeyTable, right?
`fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);`
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemV1.java
##########
@@ -0,0 +1,168 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.ozone;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Random;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Parameterized.class)
+public class TestRootedOzoneFileSystemV1 extends TestRootedOzoneFileSystem {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(TestRootedOzoneFileSystemV1.class);
+
+ @Parameterized.Parameters
+ public static Collection<Object[]> data() {
+ return Arrays.asList(
+ new Object[]{true, true},
+ new Object[]{true, false},
+ new Object[]{false, true},
+ new Object[]{false, false});
+ }
+
+ public TestRootedOzoneFileSystemV1(boolean setDefaultFs,
+ boolean enableOMRatis) throws Exception {
+ super(setDefaultFs, enableOMRatis);
+ }
+
+ @BeforeClass
+ public static void init() throws Exception {
+ setIsBucketFSOptimized(true);
+ TestRootedOzoneFileSystem.init();
+ }
+
+ /**
+ * OFS: Test recursive listStatus on root and volume.
+ */
+ @Override
+ @Ignore("TODO:HDDS-4360")
+ public void testListStatusRootAndVolumeRecursive() throws IOException {
+ }
+
+ @Test
+ public void testRenameDir() throws Exception {
Review comment:
Appreciate if you could check the feasibility of adding these new rename test cases to `TestRootedOzoneFileSystem` something similar to TestOzoneFileSystem?
If you have already tried, please point me to the test failures. Thanks!
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r585332777
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestReadRetries.java
##########
@@ -241,16 +233,6 @@ public void testPutKeyAndGetKeyThreeNodes()
factory.releaseClient(clientSpi, false);
}
- private void verifyIntermediateDir(OzoneBucket bucket,
Review comment:
Resolving this based on offline discussion
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r585332426
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1830,14 +1837,23 @@ private OzoneFileStatus getOzoneFileStatus(String volumeName,
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
volumeName, bucketName, keyName);
+ // if the queried key is already a directory key (For eg: Keyname= a/b/c/)
+ if (fileKeyBytes.endsWith(OZONE_URI_DELIMITER)){
Review comment:
Removed this check.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r585211574
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -1276,7 +1296,7 @@ public void testTrash() throws Exception {
GenericTestUtils.waitFor(
() -> getOMMetrics().getNumTrashDeletes() > prevNumTrashDeletes
&& getOMMetrics().getNumTrashFilesDeletes()
- > prevNumTrashFileDeletes, 100, 180000);
+ >= prevNumTrashFileDeletes, 100, 180000);
Review comment:
Its taken care in new commit. Thanks @sadanand48
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r585332566
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemV1.java
##########
@@ -0,0 +1,168 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.ozone;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Random;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+@RunWith(Parameterized.class)
+public class TestRootedOzoneFileSystemV1 extends TestRootedOzoneFileSystem {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(TestRootedOzoneFileSystemV1.class);
+
+ @Parameterized.Parameters
+ public static Collection<Object[]> data() {
+ return Arrays.asList(
+ new Object[]{true, true},
+ new Object[]{true, false},
+ new Object[]{false, true},
+ new Object[]{false, false});
+ }
+
+ public TestRootedOzoneFileSystemV1(boolean setDefaultFs,
+ boolean enableOMRatis) throws Exception {
+ super(setDefaultFs, enableOMRatis);
+ }
+
+ @BeforeClass
+ public static void init() throws Exception {
+ setIsBucketFSOptimized(true);
+ TestRootedOzoneFileSystem.init();
+ }
+
+ /**
+ * OFS: Test recursive listStatus on root and volume.
+ */
+ @Override
+ @Ignore("TODO:HDDS-4360")
+ public void testListStatusRootAndVolumeRecursive() throws IOException {
+ }
+
+ @Test
+ public void testRenameDir() throws Exception {
Review comment:
Added tests to TestRootedOzoneFileSystem.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r585212049
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -1257,8 +1276,9 @@ public void testTrash() throws Exception {
// This condition should pass after the checkpoint
Assert.assertTrue(getOMMetrics()
.getNumTrashRenames() > prevNumTrashRenames);
+ // With new layout version, file renames wouldn't be counted
Review comment:
Thanks for addressing the comment based on offline discussion.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r583401214
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -1257,8 +1276,9 @@ public void testTrash() throws Exception {
// This condition should pass after the checkpoint
Assert.assertTrue(getOMMetrics()
.getNumTrashRenames() > prevNumTrashRenames);
+ // With new layout version, file renames wouldn't be counted
Review comment:
Sorry, could you please give some insight on - how this function differs in old layout V0 and new layout V1 version?
Secondly, this test case will run same way in both V0 and V1, right?
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -1276,7 +1296,7 @@ public void testTrash() throws Exception {
GenericTestUtils.waitFor(
() -> getOMMetrics().getNumTrashDeletes() > prevNumTrashDeletes
&& getOMMetrics().getNumTrashFilesDeletes()
- > prevNumTrashFileDeletes, 100, 180000);
+ >= prevNumTrashFileDeletes, 100, 180000);
Review comment:
Same comment as above.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestReadRetries.java
##########
@@ -241,16 +233,6 @@ public void testPutKeyAndGetKeyThreeNodes()
factory.releaseClient(clientSpi, false);
}
- private void verifyIntermediateDir(OzoneBucket bucket,
Review comment:
Thanks @sadanand48 for the clear explanation. This test were running with below params. This gives me the impression that with existing code `enableFileSystemFlag=true`, it expects exception should be thrown for a dir.
enableFileSystemFlag=true and layoutVersion=V0
enableFileSystemFlag=true and layoutVersion=V1
I think, we need to get the correct functionality or the expectation with enableFileSystemFlag=true or false.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #1965:
URL: https://github.com/apache/ozone/pull/1965#discussion_r583080138
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestReadRetries.java
##########
@@ -241,16 +233,6 @@ public void testPutKeyAndGetKeyThreeNodes()
factory.releaseClient(clientSpi, false);
}
- private void verifyIntermediateDir(OzoneBucket bucket,
Review comment:
removed this because this expects an Exception while performing getKey on a directory which has been changed as part of this PR.
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr merged pull request #1965: HDDS-4490.[FSO]RenameAndDelete : make ofs#rename and ofs#delete an atomic operation.
Posted by GitBox <gi...@apache.org>.
rakeshadr merged pull request #1965:
URL: https://github.com/apache/ozone/pull/1965
----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org