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 2022/05/23 14:53:30 UTC
[GitHub] [ozone] nandakumar131 opened a new pull request, #3449: Add Volume and Bucket ID to the key path for FSO Objects
nandakumar131 opened a new pull request, #3449:
URL: https://github.com/apache/ozone/pull/3449
## What changes were proposed in this pull request?
Adding Volume and Bucket ID to the key path for FSO Objects
## What is the link to the Apache JIRA
HDDS-6768
## How was this patch tested?
Modified/updated the existing unit/integration tests.
--
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 diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r880265160
##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java:
##########
@@ -464,4 +474,8 @@ Set<String> getMultipartUploadKeys(String volumeName,
*/
Table<String, OmKeyInfo> getDeletedDirTable();
+ long getVolumeId(String volume) throws IOException;
Review Comment:
Please add javadoc for #getVolumeId and #getBucketId methods
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -2298,25 +2309,27 @@ private void slimLocationVersion(OmKeyInfo... keyInfos) {
}
@Override
- public OmKeyInfo getPendingDeletionDir() throws IOException {
- OmKeyInfo omKeyInfo = null;
+ public Table.KeyValue<String, OmKeyInfo> getPendingDeletionDir()
+ throws IOException {
+ // TODO: Make the return type as OmDirectoryInfo after adding
Review Comment:
Please mention the jira(create if not) to modify OmDirectoryInfo. Like we discussed the splitting logic is error prone and will refactor immediately after this patch.
--
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 diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r885742376
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -146,7 +146,12 @@ public Collection<OzoneFileStatus> listStatusFSO(OmKeyArgs args,
// fetch the db key based on parent prefix id.
long id = getId(fileStatus, omBucketInfo);
- dbPrefixKey = metadataManager.getOzonePathKey(id, "");
+ final long volumeId = metadataManager.getVolumeId(
Review Comment:
As this call is outside bucket lock, volume and bucket can be null. Need to handle that case inside #getBucketId() and #getVolumeId(). I agree to handle this in a separate jira considering managing the bulk patch.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -146,7 +146,12 @@ public Collection<OzoneFileStatus> listStatusFSO(OmKeyArgs args,
// fetch the db key based on parent prefix id.
long id = getId(fileStatus, omBucketInfo);
- dbPrefixKey = metadataManager.getOzonePathKey(id, "");
+ final long volumeId = metadataManager.getVolumeId(
+ omBucketInfo.getVolumeName());
+ final long bucketId = metadataManager.getBucketId(
Review Comment:
Here, we can get the bucketId from `omBucketInfo.getObjectId()` itself, not required to call again.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -189,8 +194,13 @@ private String getDbKey(String key, OmKeyArgs args,
null, true);
Preconditions.checkNotNull(fileStatusInfo);
startKeyParentId = getId(fileStatusInfo, omBucketInfo);
+ final long volumeId = metadataManager.getVolumeId(
+ omBucketInfo.getVolumeName());
+ final long bucketId = metadataManager.getBucketId(
+ omBucketInfo.getVolumeName(), omBucketInfo.getBucketName());
Review Comment:
Here also, we can get the bucketId from omBucketInfo.getObjectId() itself, not required to call again.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -189,8 +194,13 @@ private String getDbKey(String key, OmKeyArgs args,
null, true);
Preconditions.checkNotNull(fileStatusInfo);
startKeyParentId = getId(fileStatusInfo, omBucketInfo);
+ final long volumeId = metadataManager.getVolumeId(
+ omBucketInfo.getVolumeName());
Review Comment:
Same comment as above. This call is outside bucket lock, volume and bucket can be null. Need to handle that case inside #getBucketId() and #getVolumeId().
--
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] nandakumar131 commented on a diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r880771332
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -2298,25 +2309,27 @@ private void slimLocationVersion(OmKeyInfo... keyInfos) {
}
@Override
- public OmKeyInfo getPendingDeletionDir() throws IOException {
- OmKeyInfo omKeyInfo = null;
+ public Table.KeyValue<String, OmKeyInfo> getPendingDeletionDir()
+ throws IOException {
+ // TODO: Make the return type as OmDirectoryInfo after adding
Review Comment:
Created [HDDS-6798](https://issues.apache.org/jira/browse/HDDS-6798) for this.
--
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] nandakumar131 commented on a diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r882437934
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -261,7 +261,7 @@ message OMResponse {
optional ListTrashResponse listTrashResponse = 91;
optional RecoverTrashResponse RecoverTrashResponse = 92;
- optional PurgePathsResponse purgePathsResponse = 93;
+ optional PurgeDirectoriesRequest purgeDirectoriesResponse = 93;
Review Comment:
Thanks for catching this.
--
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] nandakumar131 commented on a diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r885831037
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -189,8 +194,13 @@ private String getDbKey(String key, OmKeyArgs args,
null, true);
Preconditions.checkNotNull(fileStatusInfo);
startKeyParentId = getId(fileStatusInfo, omBucketInfo);
+ final long volumeId = metadataManager.getVolumeId(
+ omBucketInfo.getVolumeName());
Review Comment:
Fixed by refactoring the code.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -189,8 +194,13 @@ private String getDbKey(String key, OmKeyArgs args,
null, true);
Preconditions.checkNotNull(fileStatusInfo);
startKeyParentId = getId(fileStatusInfo, omBucketInfo);
+ final long volumeId = metadataManager.getVolumeId(
+ omBucketInfo.getVolumeName());
+ final long bucketId = metadataManager.getBucketId(
+ omBucketInfo.getVolumeName(), omBucketInfo.getBucketName());
Review Comment:
Fixed.
--
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 #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on PR #3449:
URL: https://github.com/apache/ozone/pull/3449#issuecomment-1135822983
@nandakumar131 Due to the recent commit changes, following test cases are failing
```
org.apache.hadoop.ozone.om.response.key.TestOMOpenKeysDeleteResponse
org.apache.hadoop.ozone.om.request.key.TestOMOpenKeysDeleteRequest
```
--
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] nandakumar131 commented on a diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r882435889
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -183,7 +183,7 @@ message OMRequest {
optional RevokeS3SecretRequest RevokeS3SecretRequest = 93;
- optional PurgePathsRequest purgePathsRequest = 94;
Review Comment:
PurgePathRequest is refactored into an internal message inside PurgeDirectoryRequest to include volumeId and bucketId
--
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] prashantpogde commented on a diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r880922714
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -183,7 +183,7 @@ message OMRequest {
optional RevokeS3SecretRequest RevokeS3SecretRequest = 93;
- optional PurgePathsRequest purgePathsRequest = 94;
Review Comment:
Is there a reason for this rename ?
--
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] prashantpogde commented on a diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r880923440
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -261,7 +261,7 @@ message OMResponse {
optional ListTrashResponse listTrashResponse = 91;
optional RecoverTrashResponse RecoverTrashResponse = 92;
- optional PurgePathsResponse purgePathsResponse = 93;
+ optional PurgeDirectoriesRequest purgeDirectoriesResponse = 93;
Review Comment:
s/PurgeDirectoriesRequest/purgeDirectoriesResponse ??
--
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] nandakumar131 commented on pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on PR #3449:
URL: https://github.com/apache/ozone/pull/3449#issuecomment-1136210943
Created [HDDS-6797](https://issues.apache.org/jira/browse/HDDS-6797) for [PrefixFSO.md](https://github.com/apache/ozone/blob/master/hadoop-hdds/docs/content/feature/PrefixFSO.md) update.
--
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] nandakumar131 merged pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
nandakumar131 merged PR #3449:
URL: https://github.com/apache/ozone/pull/3449
--
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] nandakumar131 commented on a diff in pull request #3449: HDDS-6768. Add Volume and Bucket ID to the key path for FSO Objects
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on code in PR #3449:
URL: https://github.com/apache/ozone/pull/3449#discussion_r885824584
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -146,7 +146,12 @@ public Collection<OzoneFileStatus> listStatusFSO(OmKeyArgs args,
// fetch the db key based on parent prefix id.
long id = getId(fileStatus, omBucketInfo);
- dbPrefixKey = metadataManager.getOzonePathKey(id, "");
+ final long volumeId = metadataManager.getVolumeId(
Review Comment:
Refactored the code such that we don't need lock anymore to get _volumeId_ and _bucketId_.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java:
##########
@@ -146,7 +146,12 @@ public Collection<OzoneFileStatus> listStatusFSO(OmKeyArgs args,
// fetch the db key based on parent prefix id.
long id = getId(fileStatus, omBucketInfo);
- dbPrefixKey = metadataManager.getOzonePathKey(id, "");
+ final long volumeId = metadataManager.getVolumeId(
+ omBucketInfo.getVolumeName());
+ final long bucketId = metadataManager.getBucketId(
Review Comment:
Fixed.
--
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