HDDS-1737. Add Volume check in KeyManager and File Operations. #1559
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
/label ozone |
|
Can you please check the unit test failures? thanks |
|
Yes, I will check it soon. |
|
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Here it should be if (!omMetadataManager.getVolumeTable().isExist(volumeName)) right?
And also we should pass omMetadataManagerImpl.getVolumeKey/getBucketKey, not direct volumeName/bucketName.
As here if it does not exist, we should return error?
/**
* Check if a given key exists in Metadata store.
* (Optimization to save on data deserialization)
* A lock on the key / bucket needs to be acquired before invoking this API.
* @param key metadata key
* @return true if the metadata store contains a key.
* @throws IOException on Failure
*/
boolean isExist(KEY key) throws IOException;
There was a problem hiding this comment.
And also we can do little optimization here, check the first bucket exists or not, if it does not exist, then check volume?
There was a problem hiding this comment.
Thanks @bharatviswa504
I update the PR, and also add some test aiming to the change.
|
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
We can move this to a base class method, which can be used in all classes.
There was a problem hiding this comment.
We can move this to a base class method, which can be used every where. We already have such method in OMKeyRequest. (validateBucketAndVolume)
There was a problem hiding this comment.
Thanks @bharatviswa504
That's my fault to ignore the base class method.
I am going to update the PR.
There was a problem hiding this comment.
This can be moved to inside if (omBucketInfo == null) to Line 153.
There was a problem hiding this comment.
Thanks @bharatviswa504
I found the check also can be done by the base class method,
so I fix it and add some tests.
bharatviswa504
left a comment
bharatviswa504
left a comment
There was a problem hiding this comment.
We need a similar change in OMFileCreateRequest
|
💔 -1 overall
This message was automatically generated. |
bharatviswa504
left a comment
bharatviswa504
left a comment
There was a problem hiding this comment.
Over all LGTM. Thank You @cxorm for the update.
Can you rebase with the latest trunk, to get one final clean Jenkins runs for acceptance tests? As these are fixed in the latest trunk.
This reverts commit 369e206a3b6505013fa8b4933cd89bc6a8a4c27c.
Add some tests for the fixing of request.
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @bharatviswa504 |
|
/test |
|
Thank you very much to open this pull request. During the weekend the Ozone source code has been moved out from apache/hadoop repository to apache/hadoop-ozone repository. This git commits are rewritten, but the branch of this pull request is also transformed (state of Saturday morning), you can use the new, migrated branch to recreate this pull request. Your pull request is important for us: Can you please re-create your pull request in the new repository? 1. Create a new fork of https://github.com/apache/hadoop-ozone 2. Clone it and have both your fork and the apache repo as remotes: 3. Fetch your migrated branch and push it to your fork. 4. And create the new pull request on the new repository. If you need more information, please check this wiki page or contact with me (my github user name + apache.org). Thank you, and sorry for the inconvenience. |
Thanks @elek |
|
Migrated to apache/ozone#2 |
5 participants
NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute