-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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-1619. Support volume acl operations for OM HA. Contributed by… #1147
Conversation
💔 -1 overall
This message was automatically generated. |
...nager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
...nager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
...nager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
Show resolved
Hide resolved
...nager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerHARequestHandlerImpl.java
Show resolved
Hide resolved
...nager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
...nager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
...nager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
...nager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
...nager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
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.
I have a few comments.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
To handle Non-HA also with new HA code, we have done some changes in HDDS-1856. So, this Jira needs a few more changes like adding to double-buffer and setFlushFuture in validateAndUpdateCache. |
bq. To handle Non-HA also with new HA code, we have done some changes in HDDS-1856. So, this Jira needs a few more changes like adding to double-buffer and setFlushFuture in validateAndUpdateCache. Thanks for the heads up, @bharatviswa504 . I've rebase the change and also added removeAcl and setAcl for volume. |
💔 -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. |
...nager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
...nager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeAddAclRequest.java
Outdated
Show resolved
Hide resolved
💔 -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.
@xiaoyuyao
One comment which is missing in the PR is
We should set success with operationResult flag, because onInit() sets it to true by default.
With this, this flag will be checked as one of the condition in OMVolumeAclResponse, and do the DB Batch.
9bed6c3
to
db5f4d8
Compare
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 LGTM. Pending CI.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Thanks @bharatviswa504 for the review and discussions. I've merged the PR to trunk. |
No description provided.