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/23 15:07:36 UTC
[GitHub] [ozone] aryangupta1998 opened a new pull request #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
aryangupta1998 opened a new pull request #2561:
URL: https://github.com/apache/ozone/pull/2561
## What changes were proposed in this pull request?
In OmMetadataManagerImpl we have two function getKeyTable() and getOpenKeyTable() which uses the 'isBucketFSOptimized' flag to determine FSO buckets, with help of this Jira we will be independent of this flag and deduce bucketLayout directly from OmBucketInfo.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5636
## 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 commented on a change in pull request #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r711898329
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java
##########
@@ -1492,4 +1493,8 @@ private RequestContext currentUserReads() throws IOException {
.setAclType(ACLIdentityType.USER)
.build();
}
+
+ public BucketLayout getBucketLayout() {
Review comment:
@aryangupta1998, Adding two points:
Point-1) I agree BucketLayout.DEFAULT is correct but I'd prefer to explicitly to use `BucketLayout.LEGACY` instead of using BucketLayout.DEFAULT. My reasoning is, in future if someone changes DEFAULT value to something else, then all the test cases has to be revisited. As we know all these test cases are legacy fashion and it can be defined to `BucketLayout.LEGACY`
Point-2) Please use correct visibility either public or private. Please check all the usages of `public BucketLayout getBucketLayout() {` method in different test classes.
Also, if the above method is overridden in inherited class then please use annotation `@override` on the method.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -300,12 +301,11 @@ protected OmMetadataManagerImpl() {
return deletedDirTable;
}
- @Override
- public Table<String, OmKeyInfo> getOpenKeyTable() {
- if (OzoneManagerRatisUtils.isBucketFSOptimized()) {
- return openFileTable;
+ public Table getOpenKeyTable(BucketLayout bucketLayout) {
+ if (bucketLayout.equals(BucketLayout.OBJECT_STORE)) {
Review comment:
@aryangupta1998 thanks for the patch. Like we discussed offline to fix the test failure, could you modify the condition to include the LEGACY case.
```
if (bucketLayout.equals(BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
return openFileTable;
}
return openKeyTable;
```
##########
File path: hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -311,7 +312,14 @@ boolean recoverTrash(String volumeName, String bucketName,
*
* @return Table.
*/
- Table<String, OmKeyInfo> getOpenKeyTable();
+ //Table<String, OmKeyInfo> getOpenKeyTable();
Review comment:
Please remove the commented line.
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r719103600
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -452,15 +457,12 @@ protected void getFileEncryptionInfoForMpuKey(KeyArgs keyArgs,
acquireLock = omMetadataManager.getLock().acquireReadLock(
BUCKET_LOCK, volumeName, bucketName);
try {
-
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(
Pair.of(keyArgs.getVolumeName(), keyArgs.getBucketName()));
-
- OmKeyInfo omKeyInfo = omMetadataManager.getOpenKeyTable().get(
- omMetadataManager.getMultipartKey(resolvedBucket.realVolume(),
- resolvedBucket.realBucket(), keyArgs.getKeyName(),
- keyArgs.getMultipartUploadID()));
-
+ OmKeyInfo omKeyInfo =
+ omMetadataManager.getOpenKeyTable(getBucketLayout()).get(
Review comment:
@aryangupta1998
It should be `resolvedBucket.realVolume()` and `resolvedBucket.realBucket()`. Please modify it and is the reason for `TestOzoneAtRestEncryption#testMPUwithLinkBucket`
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r717185468
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -3110,4 +3117,8 @@ public boolean isBucketFSOptimized(String volName, String buckName)
}
return false;
}
+
+ public BucketLayout getBucketLayout() {
+ return BucketLayout.DEFAULT;
Review comment:
@aryangupta1998 Its hard coded to DEFAULT/LEGACY. This is the reason for the test case failure `TestOzoneClientMultipartUploadWithFSO.testListPartsWithPartMarkerGreaterThanPartCount`. [Failed_Build](https://github.com/apache/ozone/pull/2561/checks?check_run_id=3722349964).
getBucketLayout() has to be determined from bucket for the functions that are common to non-FSO and FSO.
Following methods in KeyManagerImpl has common for non-FSO and FSO execution path. Please modify these calls by reading bucketLayout from OmBucketInfo.
KeyManagerImpl#checkAccess() -> getBucketLayout()
and
KeyManagerImpl#listParts() -> getBucketLayout()
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r717185468
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -3110,4 +3117,8 @@ public boolean isBucketFSOptimized(String volName, String buckName)
}
return false;
}
+
+ public BucketLayout getBucketLayout() {
+ return BucketLayout.DEFAULT;
Review comment:
@aryangupta1998 Its hard coded to DEFAULT/LEGACY. This is the reason for the test case failure `TestOzoneClientMultipartUploadWithFSO.testListPartsWithPartMarkerGreaterThanPartCount`. [Failed_Build](https://github.com/apache/ozone/pull/2561/checks?check_run_id=3722349964).
getBucketLayout() has to be determined from bucket for the functions that are common to non-FSO and FSO.
Following methods in KeyManagerImpl has common for non-FSO and FSO execution path. Please modify these calls by reading bucketLayout from OmBucketInfo.
KeyManagerImpl#checkAccess() -> getBucketLayout()
and
KeyManagerImpl#listParts() -> getBucketLayout()
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r719187043
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##########
@@ -969,4 +972,8 @@ public static void validateBucket(OMMetadataManager metadataManager,
BUCKET_NOT_FOUND);
}
}
+
Review comment:
Please remove this method as no one is using 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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r716476650
##########
File path: hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -311,7 +312,14 @@ boolean recoverTrash(String volumeName, String bucketName,
*
* @return Table.
*/
- Table<String, OmKeyInfo> getOpenKeyTable();
+ //Table<String, OmKeyInfo> getOpenKeyTable();
Review comment:
Done
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java
##########
@@ -1492,4 +1493,8 @@ private RequestContext currentUserReads() throws IOException {
.setAclType(ACLIdentityType.USER)
.build();
}
+
+ public BucketLayout getBucketLayout() {
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -300,12 +301,11 @@ protected OmMetadataManagerImpl() {
return deletedDirTable;
}
- @Override
- public Table<String, OmKeyInfo> getOpenKeyTable() {
- if (OzoneManagerRatisUtils.isBucketFSOptimized()) {
- return openFileTable;
+ public Table getOpenKeyTable(BucketLayout bucketLayout) {
+ if (bucketLayout.equals(BucketLayout.OBJECT_STORE)) {
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r717920290
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -3110,4 +3117,8 @@ public boolean isBucketFSOptimized(String volName, String buckName)
}
return false;
}
+
+ public BucketLayout getBucketLayout() {
+ return BucketLayout.DEFAULT;
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r719184612
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java
##########
@@ -276,4 +278,9 @@ private void addS3MultipartUploadCommitPartResponseFSO(String volumeName,
omMetadataManager.getStore().commitBatchOperation(batchOperation);
}
+
+ @Override
Review comment:
@aryangupta1998
Please overridde getBucketLayout to FSO in S3MultipartUploadCompleteResponseWithFSO.java. This will make TestS3MultipartUploadCompleteResponseWithFSO test happy.
```
@Override
public BucketLayout getBucketLayout() {
return BucketLayout.FILE_SYSTEM_OPTIMIZED;
}
```
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#issuecomment-932123469
Thanks @aryangupta1998 for the good efforts!
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r719107758
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -452,15 +457,12 @@ protected void getFileEncryptionInfoForMpuKey(KeyArgs keyArgs,
acquireLock = omMetadataManager.getLock().acquireReadLock(
BUCKET_LOCK, volumeName, bucketName);
try {
-
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(
Pair.of(keyArgs.getVolumeName(), keyArgs.getBucketName()));
-
- OmKeyInfo omKeyInfo = omMetadataManager.getOpenKeyTable().get(
- omMetadataManager.getMultipartKey(resolvedBucket.realVolume(),
- resolvedBucket.realBucket(), keyArgs.getKeyName(),
- keyArgs.getMultipartUploadID()));
-
+ OmKeyInfo omKeyInfo =
+ omMetadataManager.getOpenKeyTable(getBucketLayout()).get(
Review comment:
Done
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java
##########
@@ -125,4 +126,9 @@ protected OmKeyInfo getOmKeyInfo() {
protected List<OmKeyInfo> getPartsUnusedList() {
return partsUnusedList;
}
+
+ @Override
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 pull request #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#issuecomment-927776986
@aryangupta1998 As BucketLayout is statically defined to BucketLayout.DEFAULT, its writing all the keys to KeyTable. But all the FSO unit tests are expecting to write files/dirs to respective `FileTable/OpenFileTable`. Thats the reason for the unit test failures.
```
TestOMRequestUtils.java
public static BucketLayout getBucketLayout() {
return BucketLayout.DEFAULT;
}
```
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r719077083
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java
##########
@@ -125,4 +126,9 @@ protected OmKeyInfo getOmKeyInfo() {
protected List<OmKeyInfo> getPartsUnusedList() {
return partsUnusedList;
}
+
+ @Override
Review comment:
@aryangupta1998 Please remove it, as its expects 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] aryangupta1998 commented on a change in pull request #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r719210754
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java
##########
@@ -276,4 +278,9 @@ private void addS3MultipartUploadCommitPartResponseFSO(String volumeName,
omMetadataManager.getStore().commitBatchOperation(batchOperation);
}
+
+ @Override
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
rakeshadr merged pull request #2561:
URL: https://github.com/apache/ozone/pull/2561
--
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 #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r719210273
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##########
@@ -969,4 +972,8 @@ public static void validateBucket(OMMetadataManager metadataManager,
BUCKET_NOT_FOUND);
}
}
+
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