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/08/14 22:59:27 UTC
[GitHub] [ozone] aryangupta1998 opened a new pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket type.
aryangupta1998 opened a new pull request #2533:
URL: https://github.com/apache/ozone/pull/2533
## What changes were proposed in this pull request?
Now we determine if the bucket is FSOptimized if the config 'ozone.om.enable.filesystem.paths' is set to true. In this and follow-ups jiras we will remove this property and determine the bucket layout by directly comparing it with an enum(BucketType.FILE_SYSTEM_OPTIMIZED).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5370
## How was this patch tested?
Tested Manually.
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr edited a comment on pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#issuecomment-899209334
@aryangupta1998 There are test case failures, due to the refactored logic of isBucketFSOptimized. Please skip the `isBucketFSOptimized` check by setting the flag to `isFSOBucketCheckEnabled` false.
On a second thought, like we discussed offline we can add a safer null check for the `ozoneManager == null` instead of using the above mentioned flag. ozoneManager can be null for non-FSO test cases and will execute non-FSO code path.
But, please ensure that all `*FSO test cases` would see a non-null ozoneManager to cover the FSO execution code path.
https://github.com/apache/ozone/pull/2533/files#diff-bde0dade7dd5ddda419499f4f999d25d40fcec1412e0ce809c36ffd1be473f22R3101
```
public boolean isBucketFSOptimized(String volName, String buckName)
throws IOException {
// safer check for unit test case. In reality ozoneManager will never be null
if (ozoneManager == null) {
return false;
}
```
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#issuecomment-899209334
@aryangupta1998 There are test case failures, due to the refactored logic of isBucketFSOptimized. Please skip the `isBucketFSOptimized` check by setting the flag to `isFSOBucketCheckEnabled` false.
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r690149178
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
Review comment:
If you agree and change code based on my above comment then you can ignore this comment:-)
These two lines are duplicated everywhere in the case statement, please move it outside the switch-case.
```
buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
bucketLayout = getBucketLayout(buckInfo);
```
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
Review comment:
How about avoiding these duplicate switch statement using following approach:
- Add new function which takes args like below:
```
private static boolean isBucketFSOptimized(String volName, String buckName,
OzoneManager ozoneManager) {
BucketLayout bucketLayout = null;
OmBucketInfo buckInfo = null;
buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
bucketLayout = getBucketLayout(buckInfo);
return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
}
```
- Call the above function from the existing switch-case like below. The same can be repeated to other bucket layout cases as well : CreateKey, CommitKey etc
```
case AllocateBlock:
keyArgs = omRequest.getAllocateBlockRequest().getKeyArgs();
if (isBucketFSOptimized(keyArgs.getVolumeName(), keyArgs.getBucketName(),
ozoneManager)) {
return new OMAllocateBlockRequestWithFSO(omRequest);
}
return new OMAllocateBlockRequest(omRequest);
case CreateKey:
keyArgs = omRequest.getCreateKeyRequest().getKeyArgs();
if (isBucketFSOptimized(keyArgs.getVolumeName(), keyArgs.getBucketName(),
ozoneManager)) {
return new OMKeyCreateRequestWithFSO(omRequest);
}
return new OMKeyCreateRequest(omRequest);
.....
....
```
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -279,6 +279,7 @@ protected OmMetadataManagerImpl() {
@Override
public Table<String, OmKeyInfo> getKeyTable() {
+ // TODO: Refactor the below function by reading bucketLayout.
Review comment:
Please raise a jira task to address this TODO for tracking it. Also, please mention the jira id here in TODO section.
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r692384544
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java
##########
@@ -146,6 +148,30 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
*/
abstract String getPath();
+ public OmBucketInfo getBucketInfo(OzoneManager ozoneManager) {
+ OmBucketInfo buckInfo = null;
+ try {
+ ObjectParser objectParser = new ObjectParser(getPath(),
+ OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY);
+
+ String volume = objectParser.getVolume();
+ String bucket = objectParser.getBucket();
+
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volume, bucket);
+
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
Review comment:
Please add log message and remove e.printStackTrace() statement.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return buckInfo;
+ }
+
+ private static BucketLayout getBucketLayout(OmBucketInfo buckInfo) {
+ if (buckInfo != null) {
+ return buckInfo.getBucketLayout();
+ } else {
+ buckInfo = null;
+ // TODO: Handle bucket validation
Review comment:
bucket null is a tricky case. Since this bucketTable is not inside BUCKET_LOCK the consistency won't be guaranteed. I think, if the bucket is coming as null then how about acquiring a BUCK_LOCK and re-read the bucket key again?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
Review comment:
@aryangupta1998
Please add log message and remove e.printStackTrace() statement.
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714706286
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return buckInfo;
+ }
+
+ private static BucketLayout getBucketLayout(OmBucketInfo buckInfo) {
+ if (buckInfo != null) {
+ return buckInfo.getBucketLayout();
+ } else {
+ buckInfo = null;
+ // TODO: Handle bucket validation
Review comment:
Done
--
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 merged pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr merged pull request #2533:
URL: https://github.com/apache/ozone/pull/2533
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r695657210
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
+ return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
+ }
+
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
Review comment:
Thanks @bharatviswa504 for the comment. Now, with this patch`#getBucketTable().get(buckKey)` is coming in the hot path for all the key requests, we thought of avoiding acquiring the costlier BUCKET_LOCK. As `BucketInfo#layout` is an immutable entity, I think `#getBucketInfo()` outside lock won't affect correctness. Does it make sense to you?
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r711940295
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -267,10 +302,13 @@ private static OMClientRequest getOMAclRequest(OMRequest omRequest) {
} else if (ObjectType.BUCKET == type) {
return new OMBucketAddAclRequest(omRequest);
} else if (ObjectType.KEY == type) {
- if (isBucketFSOptimized()){
+ OMKeyAddAclRequest aclReq = new OMKeyAddAclRequest(omRequest);
+ OmBucketInfo buckInfo = aclReq.getBucketInfo(ozoneManager);
Review comment:
@aryangupta1998 Like we discussed for other client KeyRequests, `bucketInfo` can be null. [Reference comment](https://github.com/apache/ozone/pull/2533/files#r692388618)
Instead of returning BucketInfo, you can return BucketLayout.
Just rename method `OMKeyAclRequest.getBucketInfo` to `OMKeyAclRequestgetBucketLayout` and then return Layout itself. Here, you can handle bucketInfo null check and return BucketLayout.LEGACY.
For debugging purpose, please add warn log message saying that bucket doesn't exists.
LOG.error("Bucket not found: {}/{} ", volumeName, bucketName);
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714689092
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java
##########
@@ -146,6 +153,36 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
*/
abstract String getPath();
+ public BucketLayout getBucketLayout(OzoneManager ozoneManager) {
+ BucketLayout bucketLayout = BucketLayout.LEGACY;
+ OmBucketInfo buckInfo = null;
+ try {
+ ObjectParser objectParser = new ObjectParser(getPath(),
+ OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY);
+
+ String volume = objectParser.getVolume();
+ String bucket = objectParser.getBucket();
+
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volume, bucket);
+
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ if (buckInfo == null) {
+ LOG.error("Bucket not found: {}/{} ", volume, bucket);
+ return BucketLayout.LEGACY;
+ }
+ bucketLayout = buckInfo.getBucketLayout();
+ } catch (IOException e) {
+ LOG.debug("Failed to get the value for the key: " + buckKey);
Review comment:
Please change it to` LOG.error`
`LOG.error("Failed to get bucket for the key: {}", buckKey);`
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r698051485
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java
##########
@@ -146,6 +148,30 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
*/
abstract String getPath();
+ public OmBucketInfo getBucketInfo(OzoneManager ozoneManager) {
+ OmBucketInfo buckInfo = null;
+ try {
+ ObjectParser objectParser = new ObjectParser(getPath(),
+ OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY);
+
+ String volume = objectParser.getVolume();
+ String bucket = objectParser.getBucket();
+
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volume, bucket);
+
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
Review comment:
Done
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714688483
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java
##########
@@ -146,6 +153,36 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
*/
abstract String getPath();
+ public BucketLayout getBucketLayout(OzoneManager ozoneManager) {
+ BucketLayout bucketLayout = BucketLayout.LEGACY;
+ OmBucketInfo buckInfo = null;
+ try {
+ ObjectParser objectParser = new ObjectParser(getPath(),
+ OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY);
+
+ String volume = objectParser.getVolume();
+ String bucket = objectParser.getBucket();
+
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volume, bucket);
+
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ if (buckInfo == null) {
+ LOG.error("Bucket not found: {}/{} ", volume, bucket);
+ return BucketLayout.LEGACY;
+ }
+ bucketLayout = buckInfo.getBucketLayout();
+ } catch (IOException e) {
+ LOG.debug("Failed to get the value for the key: " + buckKey);
+ }
+ } catch (OMException ome) {
+ // Handle exception
Review comment:
Please add error log message here. Its not required to throw exception explicitly and the current handling of returning default `BucketLayout.LEGACY` type is OK.
--
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 a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r690403431
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -197,4 +198,118 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
return omClientResponse;
}
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Review comment:
Can we avoid copying the same method here? We can call the new method with null bucket layout in the above method.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -197,4 +198,118 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
return omClientResponse;
}
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Review comment:
Why do we need bucketLayout for keyDelete? We deduce the bucketLayout in OzoneManagerRatisUtils and create appropriate request right . Is this method only created for unit test?
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java
##########
@@ -439,6 +441,21 @@ public static void addBucketToDB(String volumeName, String bucketName,
new CacheValue<>(Optional.of(omBucketInfo), 1L));
}
+ public static void addBucketToDB(String volumeName, String bucketName,
Review comment:
same here. Can we avoid copying the method?
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr edited a comment on pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#issuecomment-899209334
@aryangupta1998 There are test case failures, due to the refactored logic of isBucketFSOptimized. Please skip the `isBucketFSOptimized` check by setting the flag to `isFSOBucketCheckEnabled` false.
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714692304
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -140,8 +142,18 @@ public static void setBucketFSOptimized(boolean enabledFSO) {
* @return OMClientRequest
* @throws IOException
*/
- public static OMClientRequest createClientRequest(OMRequest omRequest) {
+ public static OMClientRequest createClientRequest(OMRequest omRequest,
+ OzoneManager ozoneManager) throws IOException {
Review comment:
Please remove `throws IOException` from the method signature, which is not required as we are handling exception using the BucketLayout.LEGACY, right?
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714475788
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return buckInfo;
+ }
+
+ private static BucketLayout getBucketLayout(OmBucketInfo buckInfo) {
+ if (buckInfo != null) {
+ return buckInfo.getBucketLayout();
+ } else {
+ buckInfo = null;
+ // TODO: Handle bucket validation
Review comment:
Thanks @aryangupta1998 for addressing this. I didn't think about a unit test case verifying the default `BucketLayout.LEGACY` path in my previous comment. Can we add a new test case where the bucket doesn't exists and verify it. Probably you can add a simple test in TestRootedOzoneFileSystem or TestRootedOzoneFileSystemWithFSO.
Verify creating a file or key on a non-existent bucket something like,
`bucketPath = new Path(volumePath, "invalidBucket");`
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r711931057
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return buckInfo;
+ }
+
+ private static BucketLayout getBucketLayout(OmBucketInfo buckInfo) {
+ if (buckInfo != null) {
+ return buckInfo.getBucketLayout();
+ } else {
+ buckInfo = null;
+ // TODO: Handle bucket validation
Review comment:
@aryangupta1998 Since `#validateBucketAndVolume()` check is there in every OMClientRequest , how about to return `BucketLayout.LEGACY` in case of a null bucket. For debugging purpose, please add warn log message saying that bucket doesn't exists.
`LOG.error("Bucket not found: {}/{} ", volumeName, bucketName);`
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r695657210
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
+ return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
+ }
+
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
Review comment:
Thanks @bharatviswa504 for the comment. Now, with this patch`#getBucketTable().get(buckKey)` is coming in the hot path for all the key requests, we thought of avoiding acquiring the costlier BUCKET_LOCK. As `BucketInfo#layout` is an immutable entity, I think it won't affect correctness. Does it make sense to you?
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r711914723
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
+ return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
+ }
+
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
Review comment:
Done!
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r690069615
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -3093,4 +3095,25 @@ public OmKeyInfo getPendingDeletionDir() throws IOException {
return files;
}
+
+ public boolean isBucketFSOptimized(String volName, String buckName)
+ throws IOException {
+ if (!isFSOBucketCheckEnabled) {
Review comment:
Done
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r689087702
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -281,10 +287,12 @@ private static OMClientRequest getOMAclRequest(OMRequest omRequest) {
} else if (ObjectType.BUCKET == type) {
return new OMBucketRemoveAclRequest(omRequest);
} else if (ObjectType.KEY == type) {
- if (isBucketFSOptimized()){
+ OMKeyRemoveAclRequest aclReq = new OMKeyRemoveAclRequest(omRequest);
+ OmBucketInfo buckInfo = aclReq.getBucketInfo(ozoneManager);
+ if (buckInfo.equals(BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
Review comment:
`buckInfo.equals(BucketLayout.FILE_SYSTEM_OPTIMIZED)`
There is a findbug on this statement.
Please use getLayout while comparing, spomething like `buckInfo.getLaytout().equals(BucketLayout.FILE_SYSTEM_OPTIMIZED)`
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +447,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
Review comment:
`bucketLayout.equals(BucketLayout.FILE_SYSTEM_OPTIMIZED)`, there is a findbug warn, please modify like below.
bucketLayout can be null, please do `BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout)`.
Also, please add a TODO to handle the case of null buckInfo.
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r691273138
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
Review comment:
I have made the necessary changes according to your comment below.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -279,6 +279,7 @@ protected OmMetadataManagerImpl() {
@Override
public Table<String, OmKeyInfo> getKeyTable() {
+ // TODO: Refactor the below function by reading bucketLayout.
Review comment:
Done
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714691633
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -240,11 +240,17 @@ public OMResponse handleReadRequest(OMRequest request) {
@Override
public OMClientResponse handleWriteRequest(OMRequest omRequest,
long transactionLogIndex) {
- OMClientRequest omClientRequest =
- OzoneManagerRatisUtils.createClientRequest(omRequest);
- OMClientResponse omClientResponse =
- omClientRequest.validateAndUpdateCache(getOzoneManager(),
- transactionLogIndex, ozoneManagerDoubleBuffer::add);
+ OMClientRequest omClientRequest = null;
+ OMClientResponse omClientResponse = null;
+ try {
+ omClientRequest =
+ OzoneManagerRatisUtils.createClientRequest(omRequest, impl);
+ omClientResponse = omClientRequest
+ .validateAndUpdateCache(getOzoneManager(), transactionLogIndex,
+ ozoneManagerDoubleBuffer::add);
+ } catch (IOException e) {
+ LOG.debug("Exception: " + e);
Review comment:
Since we are not throwing `OzoneManagerRatisUtils.createClientRequest` exception, we can revert this change, isn't it?
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714706396
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java
##########
@@ -146,6 +153,36 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
*/
abstract String getPath();
+ public BucketLayout getBucketLayout(OzoneManager ozoneManager) {
+ BucketLayout bucketLayout = BucketLayout.LEGACY;
+ OmBucketInfo buckInfo = null;
+ try {
+ ObjectParser objectParser = new ObjectParser(getPath(),
+ OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY);
+
+ String volume = objectParser.getVolume();
+ String bucket = objectParser.getBucket();
+
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volume, bucket);
+
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ if (buckInfo == null) {
+ LOG.error("Bucket not found: {}/{} ", volume, bucket);
+ return BucketLayout.LEGACY;
+ }
+ bucketLayout = buckInfo.getBucketLayout();
+ } catch (IOException e) {
+ LOG.debug("Failed to get the value for the key: " + buckKey);
+ }
+ } catch (OMException ome) {
+ // Handle exception
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/acl/OMKeyAclRequest.java
##########
@@ -146,6 +153,36 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
*/
abstract String getPath();
+ public BucketLayout getBucketLayout(OzoneManager ozoneManager) {
+ BucketLayout bucketLayout = BucketLayout.LEGACY;
+ OmBucketInfo buckInfo = null;
+ try {
+ ObjectParser objectParser = new ObjectParser(getPath(),
+ OzoneManagerProtocolProtos.OzoneObj.ObjectType.KEY);
+
+ String volume = objectParser.getVolume();
+ String bucket = objectParser.getBucket();
+
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volume, bucket);
+
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ if (buckInfo == null) {
+ LOG.error("Bucket not found: {}/{} ", volume, bucket);
+ return BucketLayout.LEGACY;
+ }
+ bucketLayout = buckInfo.getBucketLayout();
+ } catch (IOException e) {
+ LOG.debug("Failed to get the value for the key: " + buckKey);
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -240,11 +240,17 @@ public OMResponse handleReadRequest(OMRequest request) {
@Override
public OMClientResponse handleWriteRequest(OMRequest omRequest,
long transactionLogIndex) {
- OMClientRequest omClientRequest =
- OzoneManagerRatisUtils.createClientRequest(omRequest);
- OMClientResponse omClientResponse =
- omClientRequest.validateAndUpdateCache(getOzoneManager(),
- transactionLogIndex, ozoneManagerDoubleBuffer::add);
+ OMClientRequest omClientRequest = null;
+ OMClientResponse omClientResponse = null;
+ try {
+ omClientRequest =
+ OzoneManagerRatisUtils.createClientRequest(omRequest, impl);
+ omClientResponse = omClientRequest
+ .validateAndUpdateCache(getOzoneManager(), transactionLogIndex,
+ ozoneManagerDoubleBuffer::add);
+ } catch (IOException e) {
+ LOG.debug("Exception: " + e);
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -140,8 +142,18 @@ public static void setBucketFSOptimized(boolean enabledFSO) {
* @return OMClientRequest
* @throws IOException
*/
- public static OMClientRequest createClientRequest(OMRequest omRequest) {
+ public static OMClientRequest createClientRequest(OMRequest omRequest,
+ OzoneManager ozoneManager) throws IOException {
Review comment:
Done
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714283679
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -267,10 +302,13 @@ private static OMClientRequest getOMAclRequest(OMRequest omRequest) {
} else if (ObjectType.BUCKET == type) {
return new OMBucketAddAclRequest(omRequest);
} else if (ObjectType.KEY == type) {
- if (isBucketFSOptimized()){
+ OMKeyAddAclRequest aclReq = new OMKeyAddAclRequest(omRequest);
+ OmBucketInfo buckInfo = aclReq.getBucketInfo(ozoneManager);
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
+ return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
+ }
+
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return buckInfo;
+ }
+
+ private static BucketLayout getBucketLayout(OmBucketInfo buckInfo) {
+ if (buckInfo != null) {
+ return buckInfo.getBucketLayout();
+ } else {
+ buckInfo = null;
+ // TODO: Handle bucket validation
Review comment:
Done
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#issuecomment-925570374
+1. LGTM
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r691272865
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -197,4 +198,118 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
return omClientResponse;
}
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java
##########
@@ -439,6 +441,21 @@ public static void addBucketToDB(String volumeName, String bucketName,
new CacheValue<>(Optional.of(omBucketInfo), 1L));
}
+ public static void addBucketToDB(String volumeName, String bucketName,
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -197,4 +198,118 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
return omClientResponse;
}
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Review comment:
Yes, this method is necessary for the test. Also, we need `bucketLayout` as a parameter because we need to set the `bucketLayout` in the `OmBucketInfo`, earlier for FSO(File System Optimized) cases we were dependent on a flag `isBucketFSOptimized` but with the help of this PR, we will deduce the `bucketLayout` directly from `OmBucketInfo` hence, we need to set the appropriate `bucketLayout`. I hope this clears your doubt, if you have more questions please feel free to add them!
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
Review comment:
I have made the necessary changes according to your comment below.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -279,6 +279,7 @@ protected OmMetadataManagerImpl() {
@Override
public Table<String, OmKeyInfo> getKeyTable() {
+ // TODO: Refactor the below function by reading bucketLayout.
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
Review comment:
Thanks, @rakeshadr for this nice approach, I have made the necessary changes.
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r711931057
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return buckInfo;
+ }
+
+ private static BucketLayout getBucketLayout(OmBucketInfo buckInfo) {
+ if (buckInfo != null) {
+ return buckInfo.getBucketLayout();
+ } else {
+ buckInfo = null;
+ // TODO: Handle bucket validation
Review comment:
On a second thought, its is not required to acquire a lock here.
@aryangupta1998 Since `#validateBucketAndVolume()` check is there in every OMClientRequest , how about to return `BucketLayout.LEGACY` in case of a null bucket. For debugging purpose, please add warn log message saying that bucket doesn't exists.
`LOG.error("Bucket not found: {}/{} ", volumeName, bucketName);`
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r690149178
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
Review comment:
If you agree and change code based on my above comment then you can ignore this comment:-)
These two lines are duplicated everywhere in the case statement, please move it outside the switch-case.
```
buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
bucketLayout = getBucketLayout(buckInfo);
```
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
Review comment:
How about avoiding these duplicate switch statement using following approach:
- Add new function which takes args like below:
```
private static boolean isBucketFSOptimized(String volName, String buckName,
OzoneManager ozoneManager) {
BucketLayout bucketLayout = null;
OmBucketInfo buckInfo = null;
buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
bucketLayout = getBucketLayout(buckInfo);
return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
}
```
- Call the above function from the existing switch-case like below. The same can be repeated to other bucket layout cases as well : CreateKey, CommitKey etc
```
case AllocateBlock:
keyArgs = omRequest.getAllocateBlockRequest().getKeyArgs();
if (isBucketFSOptimized(keyArgs.getVolumeName(), keyArgs.getBucketName(),
ozoneManager)) {
return new OMAllocateBlockRequestWithFSO(omRequest);
}
return new OMAllocateBlockRequest(omRequest);
case CreateKey:
keyArgs = omRequest.getCreateKeyRequest().getKeyArgs();
if (isBucketFSOptimized(keyArgs.getVolumeName(), keyArgs.getBucketName(),
ozoneManager)) {
return new OMKeyCreateRequestWithFSO(omRequest);
}
return new OMKeyCreateRequest(omRequest);
.....
....
```
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -279,6 +279,7 @@ protected OmMetadataManagerImpl() {
@Override
public Table<String, OmKeyInfo> getKeyTable() {
+ // TODO: Refactor the below function by reading bucketLayout.
Review comment:
Please raise a jira task to address this TODO for tracking it. Also, please mention the jira id here in TODO section.
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r714668160
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +473,36 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ return buckInfo;
+ }
+
+ private static BucketLayout getBucketLayout(OmBucketInfo buckInfo) {
+ if (buckInfo != null) {
+ return buckInfo.getBucketLayout();
+ } else {
+ buckInfo = null;
+ // TODO: Handle bucket validation
Review comment:
@aryangupta1998 Please refer below unit test case and can use this kinda test to verify our code path. Since all the RPC key calls are invoked on bucket, a mock based unit test will work.
```
@Test(timeout = 300_000)
public void testRequestWithNonExistentBucket() throws Exception {
ozoneManager = Mockito.mock(OzoneManager.class);
ozoneConfiguration.set(OMConfigKeys.OZONE_OM_DB_DIRS,
folder.newFolder().getAbsolutePath());
omMetadataManager = new OmMetadataManagerImpl(ozoneConfiguration);
when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
String volumeName = "vol1";
String bucketName = "invalidBuck";
OzoneManagerProtocolProtos.OMRequest omRequest =
TestOMRequestUtils.createCompleteMPURequest(volumeName, bucketName,
"mpuKey", "mpuKeyID", new ArrayList<>());
OMClientRequest req = OzoneManagerRatisUtils.createClientRequest(omRequest, ozoneManager);
Assert.assertNotNull(req);
Assert.assertTrue("Unexpected request on invalid bucket", req instanceof S3MultipartUploadCompleteRequest);
}
```
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r689097653
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +447,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -281,10 +287,12 @@ private static OMClientRequest getOMAclRequest(OMRequest omRequest) {
} else if (ObjectType.BUCKET == type) {
return new OMBucketRemoveAclRequest(omRequest);
} else if (ObjectType.KEY == type) {
- if (isBucketFSOptimized()){
+ OMKeyRemoveAclRequest aclReq = new OMKeyRemoveAclRequest(omRequest);
+ OmBucketInfo buckInfo = aclReq.getBucketInfo(ozoneManager);
+ if (buckInfo.equals(BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
Review comment:
Done
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r691273335
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
Review comment:
Thanks, @rakeshadr for this nice approach, I have made the necessary changes.
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r690013469
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -3093,4 +3095,25 @@ public OmKeyInfo getPendingDeletionDir() throws IOException {
return files;
}
+
+ public boolean isBucketFSOptimized(String volName, String buckName)
+ throws IOException {
+ if (!isFSOBucketCheckEnabled) {
Review comment:
On a second thought, like we discussed offline we can add a safer null check for the `ozoneManager == null` instead of using the `isFSOBucketCheckEnabled` test flag. ozoneManager can be null for non-FSO test cases and will execute non-FSO code path.
But, please ensure that all `*FSO test cases` would see a non-null ozoneManager to cover the FSO execution code path.
https://github.com/apache/ozone/pull/2533/files#diff-bde0dade7dd5ddda419499f4f999d25d40fcec1412e0ce809c36ffd1be473f22R3101
```
public boolean isBucketFSOptimized(String volName, String buckName)
throws IOException {
// safer check for unit test case. In reality ozoneManager will never be null
if (ozoneManager == null) {
return false;
}
```
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -3093,4 +3095,25 @@ public OmKeyInfo getPendingDeletionDir() throws IOException {
return files;
}
+
+ public boolean isBucketFSOptimized(String volName, String buckName)
+ throws IOException {
+ if (!isFSOBucketCheckEnabled) {
Review comment:
@aryangupta1998
On a second thought, like we discussed offline we can add a safer null check for the `ozoneManager == null` instead of using the `isFSOBucketCheckEnabled` test flag. ozoneManager can be null for non-FSO test cases and will execute non-FSO code path.
But, please ensure that all `*FSO test cases` would see a non-null ozoneManager to cover the FSO execution code path.
https://github.com/apache/ozone/pull/2533/files#diff-bde0dade7dd5ddda419499f4f999d25d40fcec1412e0ce809c36ffd1be473f22R3101
```
public boolean isBucketFSOptimized(String volName, String buckName)
throws IOException {
// safer check for unit test case. In reality ozoneManager will never be null
if (ozoneManager == null) {
return false;
}
```
--
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] bharatviswa504 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r689105915
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
+ return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
+ }
+
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
+ ozoneManager.getMetadataManager().getBucketKey(volName, buckName);
+ try {
+ buckInfo =
+ ozoneManager.getMetadataManager().getBucketTable().get(buckKey);
+ } catch (IOException e) {
+ e.printStackTrace();
Review comment:
Throw exception here?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
+ return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
+ }
+
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
Review comment:
Lock held is missing here?
--
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] aryangupta1998 commented on a change in pull request #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r691272865
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -197,4 +198,118 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
return omClientResponse;
}
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java
##########
@@ -439,6 +441,21 @@ public static void addBucketToDB(String volumeName, String bucketName,
new CacheValue<>(Optional.of(omBucketInfo), 1L));
}
+ public static void addBucketToDB(String volumeName, String bucketName,
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -197,4 +198,118 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
return omClientResponse;
}
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Review comment:
Yes, this method is necessary for the test. Also, we need `bucketLayout` as a parameter because we need to set the `bucketLayout` in the `OmBucketInfo`, earlier for FSO(File System Optimized) cases we were dependent on a flag `isBucketFSOptimized` but with the help of this PR, we will deduce the `bucketLayout` directly from `OmBucketInfo` hence, we need to set the appropriate `bucketLayout`. I hope this clears your doubt, if you have more questions please feel free to add them!
--
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 #2533: HDDS-5370. [FSO] Handle OMClientRequest based on the bucket layout.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2533:
URL: https://github.com/apache/ozone/pull/2533#discussion_r711914593
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java
##########
@@ -437,4 +450,117 @@ private static ServiceException createLeaderNotReadyException(
return new ServiceException(leaderNotReadyException);
}
+ public static boolean isBucketFSOptimized(OMRequest omRequest,
+ OzoneManager ozoneManager) {
+ Type cmdType = omRequest.getCmdType();
+ BucketLayout bucketLayout = null;
+ OmBucketInfo buckInfo = null;
+ switch (cmdType) {
+ case AllocateBlock:
+ String volName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getVolumeName();
+ String buckName =
+ omRequest.getAllocateBlockRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateKey:
+ volName = omRequest.getCreateKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitKey:
+ volName = omRequest.getCommitKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCommitKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case DeleteKey:
+ volName = omRequest.getDeleteKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getDeleteKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateFile:
+ volName = omRequest.getCreateFileRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getCreateFileRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CreateDirectory:
+ volName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getVolumeName();
+ buckName =
+ omRequest.getCreateDirectoryRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case RenameKey:
+ volName = omRequest.getRenameKeyRequest().getKeyArgs().getVolumeName();
+ buckName = omRequest.getRenameKeyRequest().getKeyArgs().getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case InitiateMultiPartUpload:
+ volName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getInitiateMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CommitMultiPartUpload:
+ volName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCommitMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case AbortMultiPartUpload:
+ volName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getAbortMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ case CompleteMultiPartUpload:
+ volName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getVolumeName();
+ buckName = omRequest.getCompleteMultiPartUploadRequest().getKeyArgs()
+ .getBucketName();
+ buckInfo = getOmBucketInfo(ozoneManager, buckInfo, volName, buckName);
+ bucketLayout = getBucketLayout(buckInfo);
+ break;
+ default:
+ break;
+ }
+ return BucketLayout.FILE_SYSTEM_OPTIMIZED.equals(bucketLayout);
+ }
+
+ private static OmBucketInfo getOmBucketInfo(OzoneManager ozoneManager,
+ OmBucketInfo buckInfo, String volName, String buckName) {
+ String buckKey =
Review comment:
Thanks @bharatviswa504 for the offline discussion.
@aryangupta1998 Here all the client requests are executed in sequential model, so not required to acquire a BUCKET_LOCK. All the client requests are executed through `OzoneManagerStateMachine#runCommand` function and ensures sequential execution path.
Below is the call trace to perform OM client request operation:
`OzoneManagerStateMachine#applyTransaction -> OzoneManagerStateMachine#runCommand -> OzoneManagerRequestHandler#handleWriteRequest -> OzoneManagerRatisUtils#createClientRequest -> OzoneManagerRatisUtils#getOmBucketInfo -> omMetadataManager().getBucketTable().get(buckKey)`
@aryangupta1998 could you please add java comment about this part.
--
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