[GitHub] [ozone] rakeshadr commented on a diff in pull request #3528: HDDS-6907. OFS should create buckets with FILE_SYSTEM_OPTIMIZED layout.

[GitBox <gi...@apache.org>]

rakeshadr commented on code in PR #3528:
URL: https://github.com/apache/ozone/pull/3528#discussion_r900291520


##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -190,6 +192,12 @@ public BasicRootedOzoneClientAdapterImpl(String omHost, int omPort,
       this.configuredDnPort = conf.getInt(
           OzoneConfigKeys.DFS_CONTAINER_IPC_PORT,
           OzoneConfigKeys.DFS_CONTAINER_IPC_PORT_DEFAULT);
+
+      // Fetches the bucket layout to be used by OFS.
+      this.defaultOFSBucketLayout = BucketLayout.fromString(
+          conf.get(OzoneConfigKeys.OZONE_FS_DEFAULT_BUCKET_LAYOUT,
+              OzoneConfigKeys.OZONE_FS_DEFAULT_BUCKET_LAYOUT_DEFAULT));

Review Comment:
   Add a client side validation and throw exception if its configured a value other than FSO/LEGACY buck layouts. Exception can be IllegalArgumentException or OMException or a better one.



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java:
##########
@@ -476,6 +476,15 @@ public final class OzoneConfigKeys {
       OZONE_CLIENT_BUCKET_REPLICATION_CONFIG_REFRESH_PERIOD_DEFAULT_MS =
       300 * 1000;
 
+  public static final String OZONE_FS_DEFAULT_BUCKET_LAYOUT =

Review Comment:
   @JyotinderSingh Can we add this config in OzoneClientConfig. I could see, this class encapsulates all the ozone client configs. 
   
   ```
   /**
    * Configuration values for Ozone Client.
    */
   @ConfigGroup(prefix = "ozone.client")
   public class OzoneClientConfig {
   ```
   
   With the prefix, the config name will become  `ozone.client.fs.default.bucket.layout`
   



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -224,6 +224,8 @@ public static void initClusterAndEnv() throws IOException,
           bucketLayout.name());
     } else {
       bucketLayout = BucketLayout.LEGACY;
+      conf.set(OzoneConfigKeys.OZONE_FS_DEFAULT_BUCKET_LAYOUT,

Review Comment:
   Can we add specific test case safe guarding ofs bucket creation behavior. Probably you can add a new test class itself to verify different cases:
   
   Pre-requisite) ozone.default.bucket.layout Server defaults to OBJECT_STORE. As this should be ignored at any point of time.
   
   case-1) client side configured to empty, use default FSO. Should create buck as FSO.
   case-2) client side configured to OBS, throws Exception
   case-3) client side configured to LEGACY, ofs should create buck as LEGACY.
   case-4) client side configured to FSO, ofs should create buck as FSO.



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