-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-5011. Introduce ECReplicationConfig and Java based ReplicationConfig implementation #2068
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
|
This change looks fine to me. I just had a couple of questions inline above. |
sodonnel
left a comment
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.
These changes LGTM.
| public int hashCode() { | ||
| return Objects.hash(replicationFactor); | ||
| } | ||
| } |
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.
Do we need a toProto() method for this one like the ECReplicationConfig, or is it serialized elsewhere? We can easily add it later if we need it anyway.
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.
RatisReplictionConfig is not serialized anymore to any ***ReplicationConfig proto structure. Only the replication factor is required which can be retrieved with either casting or using the getLegacyFactor helper method.
|
Planning to merge it tomorrow as there are no more concerns. @umamaheswararao let me know if you have any comments. |
umamaheswararao
left a comment
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.
LGTM
|
Merging it now. Thanks the review @sodonnel and @umamaheswararao |
What changes were proposed in this pull request?
SCM proto file should be extended to use ECReplicationConfig which can be de-/serialized to a specific ReplicationConfiguration.
Note: this is the bare minimum version of HDDS-4882 #1973 which doesn't include the refactor of the existing proto/persistent fields but de-/serialize them to the new java pojos.
This PR is marked as draft as it depends on #2064 (this PR includes that patch, too)
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5011
How was this patch tested?
With new unit tests and also with end2end testing based on branch https://github.com/elek/ozone/tree/ec where this (and #2064) are two patches from the commit list.