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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
