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/11/13 07:19:08 UTC

[GitHub] [ozone] JyotinderSingh opened a new pull request #2837: HDDS-5959. Validate buckets accessed via OFS to be in FSO/LEGACY layout

JyotinderSingh opened a new pull request #2837:
URL: https://github.com/apache/ozone/pull/2837


   ## What changes were proposed in this pull request?
   
   Throw an exception if a bucket accessed via OFS is in OBJECT_STORE layout. Allowed layouts are FILE_SYSTEM_OPTIMIZED and LEGACY.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5959
   
   ## How was this patch tested?
   
   Related integration 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.

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


[GitHub] [ozone] rakeshadr commented on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968212570


   > Thanks for the feedback @rakeshadr @mukul1987 . I have made the relevant changes - please let me know if they are fine.
   > 
   > However currently the FS related acceptance tests are still failing due to default `OBJECT_STORE` layout behaviour of buckets configured in [ozone-default.xml#2985](https://github.com/apache/ozone/blob/c2db3776773c9a80e44a4c1dbd58d38ff6a41f9f/hadoop-hdds/common/src/main/resources/ozone-default.xml#L2985). I have tried overriding that by adding `OZONE-SITE.XML_ozone.default.bucket.layout=LEGACY` to `docker-config` and `common-config` files of the related `dist/src/main/compose/{test}/*` directories. But for some reason when the corresponding docker compose cluster is started this config not picked up - and the default config value of `OBJECT_STORE` is still being used. Any clues why this might be happening?
   I will explore this part and update to you.
   BTW, @sadanand48 do you have clues to fix this case? Thanks for your time!


-- 
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


[GitHub] [ozone] rakeshadr edited a comment on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
rakeshadr edited a comment on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968073075


   Thanks @JyotinderSingh for taking this ahead!
   
   I've gone through the test case failures quickly. Most of them are expecting FS semantics test cases and complaining about unsupported OBJECT_STORE layout.
   
   1) All the contract test cases expect FS semantics. 
   Please go through the HDDS-4513 patch, like we know earlier there was cluster side OM layout configuration, which has to be modified to FSO or LEGACY semantics with `ozone.om.enable.filesystem.paths=true`. IMHO, this will make contract tests happy.
   
   2) TestOzoneFileSystem, here you have to set `OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT=LEGACY` in the else block - Refer: [TestOzoneFileSystem.java#L164](https://github.com/apache/ozone/blob/master/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java#L164). For that, please move the below settings outside if-else block.
   
   ```
         conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT,
             bucketLayout.name());
   ```
   
   3) For the TestOzoneFsHAURLs failure, please set below configs
   ```
         conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT, LEGACY); 
         conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
   ```
   
   


-- 
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


[GitHub] [ozone] rakeshadr commented on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968073075


   Thanks @JyotinderSingh for taking this ahead!
   
   I've gone through the test case failures quickly and all the contract test cases expect FS semantics. How about setting 
   


-- 
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


[GitHub] [ozone] rakeshadr commented on a change in pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#discussion_r748728251



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
##########
@@ -226,6 +226,17 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr,
     OzoneBucket bucket;
     try {
       bucket = proxy.getBucketDetails(volumeStr, bucketStr);
+
+      // Check if bucket layout is valid, OFS buckets cannot be in
+      // OBJECT_STORE layout
+      BucketLayout bucketLayout = bucket.getBucketLayout();
+      if (bucketLayout.equals(BucketLayout.OBJECT_STORE)) {
+        throw new IllegalArgumentException(bucketLayout + " does not support" +

Review comment:
       please modify message like above.




-- 
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


[GitHub] [ozone] sadanand48 commented on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968842110


   > BTW, @sadanand48 do you have clues to fix this case? Thanks for your time!
   
   @rakeshadr , The robot test tries to create bucket via cli through `ozone bucket create `and when looking into the [code](https://github.com/apache/ozone/blob/1b7072cb106429e52ab584e17502562c02eeccd1/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java#L58) i.e in `CreateBucketHandler` we don’t obtain the default bucket type from conf instead it’s taken as OBJECT_STORE . That’s probably why it wasn’t obeying the conf.  I guess the latest change by @JyotinderSingh to explicitly create fso buckets for these tests should work.


-- 
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


[GitHub] [ozone] rakeshadr commented on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968212678


   @JyotinderSingh  Please fix TestOzoneFSInputStream case , seems it will pass with those config values.


-- 
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


[GitHub] [ozone] JyotinderSingh edited a comment on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
JyotinderSingh edited a comment on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968137051


   Thanks for the feedback @rakeshadr @mukul1987 . I have made the relevant changes - please let me know if they are fine.
   
   However currently the FS related acceptance tests are still failing due to default `OBJECT_STORE` layout behaviour of buckets configured in [ozone-default.xml#2985](https://github.com/apache/ozone/blob/c2db3776773c9a80e44a4c1dbd58d38ff6a41f9f/hadoop-hdds/common/src/main/resources/ozone-default.xml#L2985). I have tried overriding that by adding `OZONE-SITE.XML_ozone.default.bucket.layout=LEGACY` to `docker-config` and `common-config` files of the related `dist/src/main/compose/{test}/*` directories. But for some reason when the corresponding docker compose cluster is started this config not picked up - and the default config value of `OBJECT_STORE` is still being used. Any clues why this might be happening?


-- 
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


[GitHub] [ozone] JyotinderSingh commented on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968137051


   Thanks for the feedback @rakeshadr @mukul1987 . I have made the relevant changes.
   
   However currently the FS related acceptance tests are still failing due to default `OBJECT_STORE` layout behaviour of buckets configured in [ozone-default.xml#2985](https://github.com/apache/ozone/blob/c2db3776773c9a80e44a4c1dbd58d38ff6a41f9f/hadoop-hdds/common/src/main/resources/ozone-default.xml#L2985). I have tried overriding that by adding `OZONE-SITE.XML_ozone.default.bucket.layout=LEGACY` to `docker-config` and `common-config` files of the related `dist/src/main/compose/{test}/*` directories. But for some reason when the corresponding docker compose cluster is started this config not picked up - and the default config value of `OBJECT_STORE` is still being used. Any clues why this might be happening?


-- 
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


[GitHub] [ozone] rakeshadr edited a comment on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
rakeshadr edited a comment on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-968212570


   > Thanks for the feedback @rakeshadr @mukul1987 . I have made the relevant changes - please let me know if they are fine.
   > 
   > However currently the FS related acceptance tests are still failing due to default `OBJECT_STORE` layout behaviour of buckets configured in [ozone-default.xml#2985](https://github.com/apache/ozone/blob/c2db3776773c9a80e44a4c1dbd58d38ff6a41f9f/hadoop-hdds/common/src/main/resources/ozone-default.xml#L2985). I have tried overriding that by adding `OZONE-SITE.XML_ozone.default.bucket.layout=LEGACY` to `docker-config` and `common-config` files of the related `dist/src/main/compose/{test}/*` directories. But for some reason when the corresponding docker compose cluster is started this config not picked up - and the default config value of `OBJECT_STORE` is still being used. Any clues why this might be happening?
   
   I will explore this part and update to you.
   BTW, @sadanand48 do you have clues to fix this case? Thanks for your time!


-- 
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


[GitHub] [ozone] JyotinderSingh commented on pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#issuecomment-971829334


   I think most of the tests are now passing. There does seem to be a compatibility test issue though.
   the xcompat test is failing on the following test
   `xcompat-cluster-1.1.0-client-1.1.0-write :: Write Compatibility`
   The test is failing on the following command:
   `ozone fs -mkdir o3fs://bucket1.vol1/dir-1.1.0`
   This is failing due the the directory being created being in OBS fashion.
   ```
   -mkdir: Bucket: bucket1 has layout: OBJECT_STORE, which does not support file system semantics. Bucket Layout must be FILE_SYSTEM_OPTIMIZED or LEGACY.
   ```
   From my understanding - this doesn’t look like a simple fix - I believe the reason it is failing is that the old 1.1.0 client doesn’t know that the buckets created by the FS need to be in FSO layout - since we added that change in #2730.
   
   I'm not sure how we should handle 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.

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


[GitHub] [ozone] rakeshadr commented on a change in pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#discussion_r748728227



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java
##########
@@ -153,6 +153,17 @@ public BasicOzoneClientAdapterImpl(String omHost, int omPort,
     objectStore = ozoneClient.getObjectStore();
     this.volume = objectStore.getVolume(volumeStr);
     this.bucket = volume.getBucket(bucketStr);
+
+    // Check if bucket layout is valid, OFS buckets cannot be in
+    // OBJECT_STORE layout
+    BucketLayout bucketLayout = bucket.getBucketLayout();
+    if (bucketLayout.equals(BucketLayout.OBJECT_STORE)) {
+      throw new IllegalArgumentException(bucketLayout + " does not support" +

Review comment:
       @JyotinderSingh Please modify the message with bucket name, something like:
   
     ```
       throw new IllegalArgumentException("Bucket: " + bucket.getName() +" is with layout: " + bucketLayout + ", which does not support" +
             " file system semantics. Bucket Layout must be " +
             BucketLayout.FILE_SYSTEM_OPTIMIZED + " or "
             + BucketLayout.LEGACY + ".");
   ```




-- 
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


[GitHub] [ozone] mukul1987 commented on a change in pull request #2837: HDDS-5959. Handles bucket layout validation logic in ofs/o3fs client.

Posted by GitBox <gi...@apache.org>.
mukul1987 commented on a change in pull request #2837:
URL: https://github.com/apache/ozone/pull/2837#discussion_r748740925



##########
File path: hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java
##########
@@ -153,6 +153,17 @@ public BasicOzoneClientAdapterImpl(String omHost, int omPort,
     objectStore = ozoneClient.getObjectStore();
     this.volume = objectStore.getVolume(volumeStr);
     this.bucket = volume.getBucket(bucketStr);
+
+    // Check if bucket layout is valid, OFS buckets cannot be in
+    // OBJECT_STORE layout
+    BucketLayout bucketLayout = bucket.getBucketLayout();
+    if (bucketLayout.equals(BucketLayout.OBJECT_STORE)) {
+      throw new IllegalArgumentException(bucketLayout + " does not support" +

Review comment:
       I will suggest to place this in a common class like FSUtils.java.




-- 
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