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/05 06:19:47 UTC

[GitHub] [ozone] rakeshadr commented on a change in pull request #2707: HDDS-5373. [FSO] Define default bucket layout.

rakeshadr commented on a change in pull request #2707:
URL: https://github.com/apache/ozone/pull/2707#discussion_r721919109



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -145,12 +145,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     OmBucketInfo omBucketInfo = null;
     if (bucketInfo.getBucketLayout() == null || bucketInfo.getBucketLayout()
         .equals(BucketLayoutProto.LEGACY)) {
-      BucketLayout defaultType = BucketLayout.LEGACY;
-      if (StringUtils
-          .equalsIgnoreCase(OMConfigKeys.OZONE_OM_METADATA_LAYOUT_PREFIX,
-              omLayout)) {
-        defaultType = BucketLayout.FILE_SYSTEM_OPTIMIZED;
-      }
+      BucketLayout defaultType = OMConfigKeys.OZONE_BUCKET_LAYOUT_DEFAULT;

Review comment:
       Please read the value from OMConfiguration instead of reading a static value. 
   
   Below is sample code reference. 
   [OMFileCreateRequest.java#L90](https://github.com/apache/ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java#L90)
   
   In your patch, you can do `ozoneMgr.getConfiguration().getTrimmed(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT OMConfigKeys.OZONE_BUCKET_LAYOUT_DEFAULT);`

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -145,12 +145,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     OmBucketInfo omBucketInfo = null;
     if (bucketInfo.getBucketLayout() == null || bucketInfo.getBucketLayout()
         .equals(BucketLayoutProto.LEGACY)) {
-      BucketLayout defaultType = BucketLayout.LEGACY;
-      if (StringUtils
-          .equalsIgnoreCase(OMConfigKeys.OZONE_OM_METADATA_LAYOUT_PREFIX,
-              omLayout)) {
-        defaultType = BucketLayout.FILE_SYSTEM_OPTIMIZED;
-      }
+      BucketLayout defaultType = OMConfigKeys.OZONE_BUCKET_LAYOUT_DEFAULT;

Review comment:
       @JyotinderSingh  Please include test case to verify the behavior.

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java
##########
@@ -270,6 +271,11 @@ private OMConfigKeys() {
 
   public static final String OZONE_OM_METADATA_LAYOUT_PREFIX = "PREFIX";
 
+  public static final String OZONE_BUCKET_LAYOUT =
+      "ozone.bucket.layout";

Review comment:
       Can you please rename the config name to`ozone.default.bucket.type` or a better name conveying that the configurations meant for setting default value. Also, please add this config to the following files:
   
   ```
   1) hadoop-hdds/common/src/main/resources/ozone-default.xml
   2) hadoop-hdds/docs/content/feature/PrefixFSO.md
   ```




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