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/10/21 05:40:19 UTC
[GitHub] [ozone] rakeshadr commented on a change in pull request #2730: HDDS-5839. Make sure buckets created from OFS are in FILE_SYSTEM_OPTI…
rakeshadr commented on a change in pull request #2730:
URL: https://github.com/apache/ozone/pull/2730#discussion_r733333536
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemMissingParent.java
##########
@@ -104,8 +107,8 @@ public void testCloseFileWithDeletedParent() throws Exception {
// Close should throw exception, Since parent doesn't exist.
LambdaTestUtils.intercept(OMException.class,
- "Cannot create file : parent/file " + "as parent "
- + "directory doesn't exist", () -> stream.close());
+ "Failed to find parent directory of parent/file in" +
Review comment:
@JyotinderSingh
Like we discussed offline, please modify OzoneManager code by throwing an exception with below kinda message as its more meaningful to an end-user.
```
"Cannot create file : parent/file " + "as parent "
+ "directory doesn't exist", () -> stream.close());
```
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -222,6 +225,13 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
OzoneBucket bucket;
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
+ String bucketLayout = bucket.getBucketLayout().name();
+ if (!StringUtils.equalsIgnoreCase(bucketLayout,
+ OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED)) {
Review comment:
@JyotinderSingh
There is a case on upgrade path. LEGACY represents a bucket without (empty/null) layout. What if a user performs operation on old buckets(LEGACY layout), presently it will fail. Since we/ozone don't have a layout conversion tool yet, how about to modify the validation by specifically checking OBJECT_STORE?
```
if (StringUtils.equalsIgnoreCase(bucketLayout, BucketLayout.OBJECT_STORE.name())) {
// throw exception.
}
```
I hope, some of the test case failures will get fixed with this change.
##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -222,6 +225,13 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
OzoneBucket bucket;
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
+ String bucketLayout = bucket.getBucketLayout().name();
+ if (!StringUtils.equalsIgnoreCase(bucketLayout,
+ OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED)) {
+ throw new IllegalArgumentException(bucketLayout + " does not support" +
+ " file system semantics. Bucket Layout must be " +
+ OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED);
Review comment:
@JyotinderSingh
Can you rename `OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED` to `OMConfigKeys.OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED`. Its unrelated to this patch but its a simple change to incorporate, I think. 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
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