-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDDS-1723. Create new OzoneManagerLock class. #1006
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
87009fb
to
c61cf0d
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, it looks excellent. Some very minor comments inline. I am +1 after these comments are looked at.
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
c61cf0d
to
3ed81da
Compare
Thank You @anuengineer for the review. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Test failures are not related to this patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, Thank you for getting this done. I have some minor comments, nothing important. You might want to see if @arp7 or @nandakumar131 wants to comment and wait for a day before commtting.
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java
Outdated
Show resolved
Hide resolved
Thank You @anuengineer for the review. |
a5b146e
to
497b151
Compare
497b151
to
919dc4c
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, I will commit this patch now. Thanks for the contribution.
* HDDS-1723. Create new OzoneManagerLock class. * add logs * fix exception message * add license * fix findbug * throw exception when acquiring 2nd lock failed. * fix review comments * fix missed review comment * fix checkstyle * fix review comments
… of metadata resources (apache#1006) * Implement MetadataResourceLoader to encapsulate loading of metadata resources. * Rename MetadataResourceLoader to MetadataResourceManager * Rename to MetadataResourceUtil and remove unused methods. * Remove any API changes that refer to future PRs.
* HDDS-1723. Create new OzoneManagerLock class. * add logs * fix exception message * add license * fix findbug * throw exception when acquiring 2nd lock failed. * fix review comments * fix missed review comment * fix checkstyle * fix review comments
Thank You @anuengineer for offline discussion and help during the code of using Short and bit manipulation, instead of BitSet.
If someone is interested in how the code looks like with BitSet, refer below link.
https://github.com/bharatviswa504/hadoop-1/blob/bhcode-anucode/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock1.java