[GitHub] [ozone] umamaheswararao commented on a change in pull request #1973: HDDS-4882. Modify proto file with container metadata and replication config

[GitBox <gi...@apache.org>]

umamaheswararao commented on a change in pull request #1973:
URL: https://github.com/apache/ozone/pull/1973#discussion_r585822299



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto
##########
@@ -125,10 +125,17 @@ enum Status {
 message AllocateScmBlockRequestProto {
   required uint64 size = 1;
   required uint32 numBlocks = 2;
+
+  //deprected, use RatisReplicationConfig

Review comment:
       deprected  -- > deprecated
   
   I think let's discuss this separately in master as it's making some deprecations?
   I suggest first let's discuss how we pass the ReplicationConfig to server from createKeyAPI:
   It should be a same object type isn't it?
   If we have two separated config classes (RatisReplicationConfig and ECReplicationConfig), then it's not easy to pass through the same API right?
   
   Example: here is the place we build OmKeyArgs and pass, here we are passing ReplicationFactor. Now having two classes, how we can use them here?
   
   Also currently replicationFactor is carrying two ReplicationType factor ( One is for NON Ratis and other is for Ratis). So, we need another config class for non Ratis ( you named only RatisReplicationConfig, why do we need to bother Ratis or non Ratis? it's just replication right )?
   
   I believe we need more discussion here.
   I think we can bring ReplicationConfig changes in master itself and in EC branch we just need to add new EC config.
   
   ECReplicationConfig: based on this name, we accepted the terminology Replication for EC as well. So, why can't we add replication type with more specific like EC_3_2 and replication factor is 5.
   Then based on replication type EC_3_2, we can define EC specific interpreter class to understand what's the parity and data block numbers? 
    
   




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org