rakeshadr commented on a change in pull request #2707:
URL: https://github.com/apache/ozone/pull/2707#discussion_r722179962
##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -2988,4 +2988,13 @@
OM/SCM/DN/S3GATEWAY Server connection timeout in milliseconds.
</description>
</property>
+
+ <property>
+ <name>ozone.default.bucket.layout</name>
+ <value>OBS</value>
Review comment:
Please use full name `OBJECT_STORE` instead of OBS.
[BucketLayout.java#L30](https://github.com/apache/ozone/blob/master/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketLayout.java#L30)
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java
##########
@@ -270,6 +270,13 @@ private OMConfigKeys() {
public static final String OZONE_OM_METADATA_LAYOUT_PREFIX = "PREFIX";
+ // Defines the default bucket behaviour when OM does not pass the
Review comment:
please modify comment to:
`Default bucket layout used by Ozone Manager during bucket creation when a
client does not specify the bucket layout option.`
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -145,10 +145,14 @@ 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)) {
+ // Bucket Layout argument was not passed during bucket creation.
Review comment:
We can simply read the default value and assign to the bucket. Please
remove the below if/else, which has some confusion.
```
// Bucket Layout argument was not passed during bucket creation.
BucketLayout defaultType;
String omDefaultBucketLayout = ozoneManager.getOMDefaultBucketLayout();
if (StringUtils.equalsIgnoreCase(
OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT_DEFAULT,
omDefaultBucketLayout)) {
defaultType = BucketLayout.OBJECT_STORE;
} else {
defaultType = BucketLayout.FILE_SYSTEM_OPTIMIZED;
```
Here the code can be as simple like,
```
if (bucketInfo.getBucketLayout() == null || bucketInfo.getBucketLayout()
.equals(BucketLayoutProto.LEGACY)) {
// Bucket Layout argument is not passed during bucket
creation, will assign OM default layout.
String omDefaultBucketLayout =
ozoneManager.getOMDefaultBucketLayout();
BucketLayout defaultType = BucketLayout.fromString(String
value);
omBucketInfo = OmBucketInfo.getFromProtobuf(bucketInfo,
defaultType);
}
```
Note: You have to implement `BucketLayout.fromString(String value);`, where
you can do a string comparison and return matching enum type.
if (StringUtils.equalsIgnoreCase(
defaultBucketLayout, BucketLayout.OBJECT_STORE.name())) {
defaultType = BucketLayout.OBJECT_STORE;
} else if (StringUtils.equalsIgnoreCase(
defaultBucketLayout, BucketLayout.FILE_SYSTEM_OPTIMIZED.name())) {
defaultType = BucketLayout.FILE_SYSTEM_OPTIMIZED;
} else {
// Todo: need to discuss about throwing a validation error during
OM startup or bucket creation time.
defaultType = BucketLayout.LEGACY;
}
##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -2988,4 +2988,13 @@
OM/SCM/DN/S3GATEWAY Server connection timeout in milliseconds.
</description>
</property>
+
+ <property>
+ <name>ozone.default.bucket.layout</name>
+ <value>OBS</value>
+ <tag>OZONE, MANAGEMENT</tag>
+ <description>
+ Default bucket layout used during bucket creation when OM does not pass
the bucket type argument.
Review comment:
small typo:
Please modify description to ->
`Default bucket layout used by Ozone Manager during bucket creation when a
client does not specify the bucket layout option.`
##########
File path: hadoop-hdds/docs/content/feature/PrefixFSO.md
##########
@@ -72,4 +72,17 @@ By default the feature is disabled. It can be enabled with
the following
<name>ozone.om.metadata.layout</name>
<value>PREFIX</value>
</property>
+```
+
+In reference to efforts towards supporting protocol aware buckets within a
+single OM the following configurations can be used to define the default value
+for bucket layout during bucket creation if the client (S3 or o3fs) is not
Review comment:
small typo:
I think to remove `(S3 or o3fs)`, let it be generic.
##########
File path: hadoop-hdds/docs/content/feature/PrefixFSO.md
##########
@@ -72,4 +72,17 @@ By default the feature is disabled. It can be enabled with
the following
<name>ozone.om.metadata.layout</name>
<value>PREFIX</value>
</property>
+```
+
+In reference to efforts towards supporting protocol aware buckets within a
+single OM the following configurations can be used to define the default value
+for bucket layout during bucket creation if the client (S3 or o3fs) is not
+passing the bucket layout argument.
+
+By default, the buckets will default to OBS behaviour.
+```XML
+<property>
+ <name>ozone.default.bucket.layout</name>
+ <value>OBS</value>
Review comment:
Like my earlier comment, please use full name OBJECT_STORE instead of
OBS.
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java
##########
@@ -270,6 +270,13 @@ private OMConfigKeys() {
public static final String OZONE_OM_METADATA_LAYOUT_PREFIX = "PREFIX";
+ // Defines the default bucket behaviour when OM does not pass the
+ // bucket layout argument during bucket creation.
+ public static final String OZONE_DEFAULT_BUCKET_LAYOUT =
+ "ozone.default.bucket.layout";
+ public static final String OZONE_DEFAULT_BUCKET_LAYOUT_DEFAULT = "OBS";
Review comment:
Like my earlier comment, please use full name OBJECT_STORE instead of
OBS and FILE_SYSTEM_OPTIMIZED instead of OBS.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
