-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDDS-1393. Convert all OM Bucket related operations to HA model. #704
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
Conversation
|
💔 -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.
Here it should be !isRatisEnabled
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.
Here it should be if Ratis is Enabled, we should call applyTransaction to persist this info to DB.
As we have not separated this into 2 phase we should do this.
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.
BucketArgs and BucketInfo have a lot of common fields. Can we combine them into one proto?
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.
Addressed for now as making bucketArgs as optional, so that we will not set this during creating OMRequest in startTransaction, and we don't write duplicate data into raft log.
Added a TODO for the suggested, as discussed offline.
6427c22 to
f5aa8ea
Compare
|
Thank You @hanishakoneru for the review. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
1708528 to
b00e8a9
Compare
|
Thank You @hanishakoneru for the review. |
|
💔 -1 overall
This message was automatically generated. |
|
LGTM. +1 pending CI. |
|
Acceptance tests of S3 tests suite are passing locally, I think cluster start might not be done so all tests failed during suite test setup stage. And test failure is not related to this patch. I will commit this shortly. |
md file for quick start. Author: xinyuiscool <[email protected]> Reviewers: Jagadish<[email protected]> Closes apache#704 from xinyuiscool/SAMZA-1911
No description provided.