[GitHub] [ozone] sodonnel commented on a change in pull request #2068: HDDS-5011. Introduce ECReplicationConfig and Java based ReplicationConfig implementation

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

sodonnel commented on a change in pull request #2068:
URL: https://github.com/apache/ozone/pull/2068#discussion_r604118294



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto
##########
@@ -132,9 +132,13 @@ message AllocateScmBlockRequestProto {
   required uint64 size = 1;
   required uint32 numBlocks = 2;
   required ReplicationType type = 3;
-  required hadoop.hdds.ReplicationFactor factor = 4;
+  optional hadoop.hdds.ReplicationFactor factor = 4;

Review comment:
       Is moving this field from required to optional OK from a forward / backwards compatibility point of view? Would an old client which has it set to required, complain if it received a message missing the field?
   
   Old Client -> New Server:
   Will only ever send the factor field | will be OK
   
   New Server -> Old Client
   Client can never understand EC. Other files would have factor set.
   
   New Client -> Old Server
   Old server cannot understand EC, must always send Factor for Ratis files.
   
   Old Server -> New Client:
   Will always sent factor
   
   I think it will be OK, as there is a natural incompatibility between old client / server and new client / server which supports EC.




-- 
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