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 2020/08/27 21:34:08 UTC
[GitHub] [hadoop-ozone] bharatviswa504 opened a new pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
bharatviswa504 opened a new pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361
## What changes were proposed in this pull request?
Disallow creation/complete MPU when a directory already exists with same name.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4155
## How was this patch tested?
Added integration tests.
Tested with S3 with this flag manually. Tests for this with enable will be added as part of HDDS-4154
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485756050
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
You mean use OzoneBucketAPI and create a Key or add a robot test using S3
If robot test, we don't have test suite with this flag enabled, opened a Jira
https://issues.apache.org/jira/browse/HDDS-4154
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485020859
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -158,6 +159,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ // Check by the time we commit this file, is there any directory
Review comment:
@bharatviswa504 can you please reword this comment? I couldn't understand what it is saying.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485022218
##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
##########
@@ -197,6 +198,18 @@ public Response put(
.build();
} catch (IOException ex) {
LOG.error("Exception occurred in PutObject", ex);
+ if (ex instanceof OMException) {
+ if (((OMException) ex).getResult() == ResultCodes.NOT_A_FILE) {
+ OS3Exception os3Exception = S3ErrorTable.newError(INVALID_REQUEST,
+ keyPath);
+ os3Exception.setErrorMessage("An error occurred (InvalidRequest) " +
Review comment:
Nice detailed explanation, thanks for adding this!
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 merged pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r487246329
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485756050
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
You mean use OzoneBucket or add a robot test using S3.
If robot test, we don't have test suite with this flag enabled, opened a jira
https://issues.apache.org/jira/browse/HDDS-4154
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485344203
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -209,6 +215,94 @@ public void testObjectStoreCreateWithO3fs() throws Exception {
}
+
+ @Test
+ public void testKeyCreationFailDuetoDirectoryCreationBeforeCommit()
+ throws Exception {
+ OzoneVolume ozoneVolume =
+ cluster.getRpcClient().getObjectStore().getVolume(volumeName);
+
+ OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName);
+
+ OzoneOutputStream ozoneOutputStream =
+ ozoneBucket.createKey("/a/b/c", 10);
+ byte[] b = new byte[10];
+ Arrays.fill(b, (byte)96);
+ ozoneOutputStream.write(b);
+
+ // Before close create directory with same name.
+ o3fs.mkdirs(new Path("/a/b/c"));
Review comment:
Added the test.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485315969
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -158,6 +159,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ // Check by the time we commit this file, is there any directory
Review comment:
done
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485021796
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -158,6 +159,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ // Check by the time we commit this file, is there any directory
+ // created if then fail it. This is being done for correctness.
+
+ // For now we are taking this approach similar to local file creation.
+ // TODO: In future this can be revisited to come up with an advanced
+ // approach.
+
+
+ if (ozoneManager.getEnableFileSystemPaths()) {
+ if (checkDirectoryAlreadyExists(volumeName, bucketName, keyName,
Review comment:
Does the directory name have a trailing `/` in RocksDB? If so, do we need to append it to keyname before calling `checkDirectoryAlreadyExists`?
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485756050
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
You mean use OzoneBucketAPI and create a Key or add a robot test using S3
If robot test, we don't have a test suite with this flag enabled, there is already an open Jira for this, we can cover this additional scenario in that.
https://issues.apache.org/jira/browse/HDDS-4154
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r487246329
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485738948
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Thanks for the additional test. Can we also add a test case that exercises the REST API to create a file and dir with the same name, in both orders?
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485339793
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -209,6 +215,94 @@ public void testObjectStoreCreateWithO3fs() throws Exception {
}
+
+ @Test
+ public void testKeyCreationFailDuetoDirectoryCreationBeforeCommit()
+ throws Exception {
+ OzoneVolume ozoneVolume =
+ cluster.getRpcClient().getObjectStore().getVolume(volumeName);
+
+ OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName);
+
+ OzoneOutputStream ozoneOutputStream =
+ ozoneBucket.createKey("/a/b/c", 10);
+ byte[] b = new byte[10];
+ Arrays.fill(b, (byte)96);
+ ozoneOutputStream.write(b);
+
+ // Before close create directory with same name.
+ o3fs.mkdirs(new Path("/a/b/c"));
Review comment:
>Do we have a test case that tests the reverse scenario? i.e. create the dir first, then try to create a file with the same >name
No such test is currently available. Will post shortly this test also.
>Also are we testing this via the object store interface with the flag enabled?
Yes test is with flag enabled.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -209,6 +215,94 @@ public void testObjectStoreCreateWithO3fs() throws Exception {
}
+
+ @Test
+ public void testKeyCreationFailDuetoDirectoryCreationBeforeCommit()
+ throws Exception {
+ OzoneVolume ozoneVolume =
+ cluster.getRpcClient().getObjectStore().getVolume(volumeName);
+
+ OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName);
+
+ OzoneOutputStream ozoneOutputStream =
+ ozoneBucket.createKey("/a/b/c", 10);
+ byte[] b = new byte[10];
+ Arrays.fill(b, (byte)96);
+ ozoneOutputStream.write(b);
+
+ // Before close create directory with same name.
+ o3fs.mkdirs(new Path("/a/b/c"));
Review comment:
>Do we have a test case that tests the reverse scenario? i.e. create the dir first, then try to create a file with the same >name
No such test is currently available. Will post shortly this test also.
>Also are we testing this via the object store interface with the flag enabled?
Yes test is with flag enabled.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485293304
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -158,6 +159,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ // Check by the time we commit this file, is there any directory
+ // created if then fail it. This is being done for correctness.
+
+ // For now we are taking this approach similar to local file creation.
+ // TODO: In future this can be revisited to come up with an advanced
+ // approach.
+
+
+ if (ozoneManager.getEnableFileSystemPaths()) {
+ if (checkDirectoryAlreadyExists(volumeName, bucketName, keyName,
Review comment:
>Does the directory name have a trailing / in RocksDB?
Yes.
>If so, do we need to append it to keyname before calling checkDirectoryAlreadyExists?
That is taken care of by the implementation of checkDirectoryAlreadyExists.
Where it uses getOzoneDirKey which appends "/"
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -158,6 +159,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ // Check by the time we commit this file, is there any directory
+ // created if then fail it. This is being done for correctness.
+
+ // For now we are taking this approach similar to local file creation.
+ // TODO: In future this can be revisited to come up with an advanced
+ // approach.
+
+
+ if (ozoneManager.getEnableFileSystemPaths()) {
+ if (checkDirectoryAlreadyExists(volumeName, bucketName, keyName,
Review comment:
>Does the directory name have a trailing / in RocksDB?
Yes.
>If so, do we need to append it to keyname before calling checkDirectoryAlreadyExists?
That is taken care of by the implementation of checkDirectoryAlreadyExists.
Where it uses getOzoneDirKey which appends "/"
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -158,6 +159,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ // Check by the time we commit this file, is there any directory
+ // created if then fail it. This is being done for correctness.
+
+ // For now we are taking this approach similar to local file creation.
+ // TODO: In future this can be revisited to come up with an advanced
+ // approach.
+
+
+ if (ozoneManager.getEnableFileSystemPaths()) {
+ if (checkDirectoryAlreadyExists(volumeName, bucketName, keyName,
Review comment:
>Does the directory name have a trailing / in RocksDB?
Yes.
>If so, do we need to append it to keyname before calling checkDirectoryAlreadyExists?
That is taken care of by the implementation of checkDirectoryAlreadyExists.
Where it uses getOzoneDirKey which appends "/"
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485293304
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -158,6 +159,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ // Check by the time we commit this file, is there any directory
+ // created if then fail it. This is being done for correctness.
+
+ // For now we are taking this approach similar to local file creation.
+ // TODO: In future this can be revisited to come up with an advanced
+ // approach.
+
+
+ if (ozoneManager.getEnableFileSystemPaths()) {
+ if (checkDirectoryAlreadyExists(volumeName, bucketName, keyName,
Review comment:
>Does the directory name have a trailing / in RocksDB?
Yes.
That is taken care of by the implementation of checkDirectoryAlreadyExists.
Where it uses getOzoneDirKey which appends "/"
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#issuecomment-689278088
Thank You @arp7 for the review.
I have addressed your review comments.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r485332430
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -209,6 +215,94 @@ public void testObjectStoreCreateWithO3fs() throws Exception {
}
+
+ @Test
+ public void testKeyCreationFailDuetoDirectoryCreationBeforeCommit()
+ throws Exception {
+ OzoneVolume ozoneVolume =
+ cluster.getRpcClient().getObjectStore().getVolume(volumeName);
+
+ OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName);
+
+ OzoneOutputStream ozoneOutputStream =
+ ozoneBucket.createKey("/a/b/c", 10);
+ byte[] b = new byte[10];
+ Arrays.fill(b, (byte)96);
+ ozoneOutputStream.write(b);
+
+ // Before close create directory with same name.
+ o3fs.mkdirs(new Path("/a/b/c"));
Review comment:
Do we have a test case that tests the reverse scenario? i.e. create the dir first, then try to create a file with the same name. Also are we testing this via the object store interface with the flag enabled?
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r487246329
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#issuecomment-692240671
Thank You @arp7 for the review.
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r487246329
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1361: HDDS-4155. Directory and filename can end up with same name in a path.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1361:
URL: https://github.com/apache/hadoop-ozone/pull/1361#discussion_r487246329
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFSWithObjectStoreCreate.java
##########
@@ -303,6 +304,12 @@ public void testMPUFailDuetoDirectoryCreationBeforeComplete()
}
+ @Test(expected = FileAlreadyExistsException.class)
Review comment:
Added a test with object store API for the same scenario
----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org