maobaolong commented on pull request #920:
URL: https://github.com/apache/hadoop-ozone/pull/920#issuecomment-629731098


   @xiaoyuyao Thank you for your review, I think this PR is significant. 
   
   > have you consider adding the additional factors into existing Enum (e.g. 
odd number < 32)
   
   Yeah, this is my first thought, but i change my mind quickly, because, i 
think we should use configuration to limit the maximum number of replication 
factor instead of the Enum in the proto. We can imagine that each time we add 
the additional refactor, we should modify the protocol, and now we support ONE 
and THREE, after this PR, we support ONE, THREE,....THIRTY-ONE, some day, ratis 
support factor TWO, we should add TWO in the factor, and the client must be 
upgraded each time. Another things, many factor in a enum make the source code 
not pretty.
   
   About your thoughts
   > 1. We won't be able to support large replication factor or arbitrary 
replication factor (even) in near future due to raft limitations.
   
   I think some of even factor like TWO will be support recently. we have 
strong request to have this feature, and the concept of RAFT support TWO member 
group, @runzhiwang as a developer of ratis is working on TWO member group ratis 
now
   
   > 2. We will have better control over the replication factor that we can 
support.
   
   Yeah, but I think we can have a support list in configuration, we can update 
our support list but no need to update source code and without client upgrade.
   
   > 3. We will need to maintain backward and forward compatibility for future 
changes.
   
   Sure, it is necessary, i see a PR to bring a maven plugin to check the proto 
compatibility, it is great.
   
   As a conclusion, i want to remove the limitation by Enum  type in proto, 
update the proto, the client  have to upgrade. We really cannot expect a 
totally factor of replication we will support future, but we can do some 
efforts like this PR to update the support replication factor without client 
upgrade  through move the concept of replication factor limitation from Enum to 
configuration.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to