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/10/19 16:05:19 UTC
[GitHub] [ozone] aryangupta1998 opened a new pull request #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
aryangupta1998 opened a new pull request #2751:
URL: https://github.com/apache/ozone/pull/2751
## What changes were proposed in this pull request?
In OmMetadataManagerImpl we have a function getKeyTable() 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.
This task is the second part and will modify the rest of the classes.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5835
## 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] aryangupta1998 commented on a change in pull request #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r737820298
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3666,4 +3668,8 @@ public void testHeadObject() throws IOException {
}
}
+
+ private BucketLayout getBucketLayout() {
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
rakeshadr merged pull request #2751:
URL: https://github.com/apache/ozone/pull/2751
--
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r734685650
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3666,4 +3668,8 @@ public void testHeadObject() throws IOException {
}
}
+
+ private BucketLayout getBucketLayout() {
Review comment:
I think we can add a comment for `getBucketLayout()` at least in a few places like OmKeyRequest.java stating that since this is a Non-FSO code path, Bucket.DEFAULT is returned which would correspond to the KeyTable since this method is mostly called from getKeyTable(BucketLayout).
--
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#issuecomment-953436050
+1 LGTM, I will merge it shortly.
Thanks @aryangupta1998 for the continuous efforts.
Thanks @sadanand48 for the reviews.
--
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
aryangupta1998 commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r736417058
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3815,7 +3815,7 @@ void reloadOMState(long newSnapshotIndex, long newSnapshotTermIndex)
metrics.setNumBuckets(metadataManager.countRowsInTable(metadataManager
.getBucketTable()));
metrics.setNumKeys(metadataManager.countEstimatedRowsInTable(metadataManager
- .getKeyTable()));
+ .getKeyTable(getBucketLayout())));
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r737619082
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3666,4 +3668,8 @@ public void testHeadObject() throws IOException {
}
}
+
+ private BucketLayout getBucketLayout() {
Review comment:
@aryangupta1998 can you please take care. Thanks!
--
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r733443091
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3815,7 +3815,7 @@ void reloadOMState(long newSnapshotIndex, long newSnapshotTermIndex)
metrics.setNumBuckets(metadataManager.countRowsInTable(metadataManager
.getBucketTable()));
metrics.setNumKeys(metadataManager.countEstimatedRowsInTable(metadataManager
- .getKeyTable()));
+ .getKeyTable(getBucketLayout())));
Review comment:
@aryangupta1998 , Need to include both FSO and OBS key paths. Presently, this patch is considering only DEFAULT layout.
```
metrics.setNumKeys(metadataManager.countEstimatedRowsInTable(metadataManager
.getKeyTable(getBucketLayout())));
```
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/FileSizeCountTask.java
##########
@@ -73,7 +74,8 @@ public FileSizeCountTask(FileCountBySizeDao fileCountBySizeDao,
*/
@Override
public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
- Table<String, OmKeyInfo> omKeyInfoTable = omMetadataManager.getKeyTable();
+ Table<String, OmKeyInfo> omKeyInfoTable =
Review comment:
@aryangupta1998 same comment here as well, we need to include both FSO and OBS key paths.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java
##########
@@ -159,8 +160,8 @@ public Response getKeysForContainer(
// Directly calling get() on the Key table instead of iterating since
// only full keys are supported now. When we change to using a prefix
// of the key, this needs to change to prefix seek.
- OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().getSkipCache(
- containerKeyPrefix.getKeyPrefix());
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable(getBucketLayout())
Review comment:
@aryangupta1998 , Here also need to include both FSO and OBS key paths. Presently, this patch is considering only DEFAULT layout.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java
##########
@@ -77,7 +78,8 @@ public ContainerKeyMapperTask(ReconContainerMetadataManager
reconContainerMetadataManager
.reinitWithNewContainerDataFromOm(new HashMap<>());
- Table<String, OmKeyInfo> omKeyInfoTable = omMetadataManager.getKeyTable();
+ Table<String, OmKeyInfo> omKeyInfoTable =
+ omMetadataManager.getKeyTable(getBucketLayout());
Review comment:
@aryangupta1998 same comment here as well, we need to include both FSO and OBS key paths.
--
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r734685650
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3666,4 +3668,8 @@ public void testHeadObject() throws IOException {
}
}
+
+ private BucketLayout getBucketLayout() {
Review comment:
I think we can add a javadoc/comment for `getBucketLayout()` at least in a few places like OmKeyRequest.java stating that since this is a Non-FSO code path, Bucket.DEFAULT is returned which would correspond to the KeyTable since this method is mostly called from getKeyTable(BucketLayout).
--
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r737614346
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java
##########
@@ -77,7 +78,8 @@ public ContainerKeyMapperTask(ReconContainerMetadataManager
reconContainerMetadataManager
.reinitWithNewContainerDataFromOm(new HashMap<>());
- Table<String, OmKeyInfo> omKeyInfoTable = omMetadataManager.getKeyTable();
+ Table<String, OmKeyInfo> omKeyInfoTable =
+ omMetadataManager.getKeyTable(getBucketLayout());
Review comment:
created task HDDS-5904 to address this comment
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/FileSizeCountTask.java
##########
@@ -73,7 +74,8 @@ public FileSizeCountTask(FileCountBySizeDao fileCountBySizeDao,
*/
@Override
public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
- Table<String, OmKeyInfo> omKeyInfoTable = omMetadataManager.getKeyTable();
+ Table<String, OmKeyInfo> omKeyInfoTable =
Review comment:
created task HDDS-5904 to address this comment
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java
##########
@@ -159,8 +160,8 @@ public Response getKeysForContainer(
// Directly calling get() on the Key table instead of iterating since
// only full keys are supported now. When we change to using a prefix
// of the key, this needs to change to prefix seek.
- OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().getSkipCache(
- containerKeyPrefix.getKeyPrefix());
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable(getBucketLayout())
Review comment:
created task HDDS-5904 to address this comment
--
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 #2751: HDDS-5835. Follow up task to use bucket layout specific DB table in OmMetadataManager.getKeyTable() - part2
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2751:
URL: https://github.com/apache/ozone/pull/2751#discussion_r734685650
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3666,4 +3668,8 @@ public void testHeadObject() throws IOException {
}
}
+
+ private BucketLayout getBucketLayout() {
Review comment:
I think we can add a javadoc for `getBucketLayout()` at least in a few places like OmKeyRequest.java stating that since this is a Non-FSO code path, Bucket.DEFAULT is returned which would correspond to the KeyTable since this method is mostly called from getKeyTable(BucketLayout).
--
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