You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/06/10 17:27:10 UTC

[GitHub] [ozone] umamaheswararao commented on a diff in pull request #3498: HDDS-6841. EC: Validate the server default configuration on Ozone manager startup

umamaheswararao commented on code in PR #3498:
URL: https://github.com/apache/ozone/pull/3498#discussion_r894753237


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -608,6 +612,43 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption)
     }
   }
 

Review Comment:
   There is a method getDefaultReplicationConfig, which is already parsing and getting the repConfig.
   Probably we can just use that method and then validate that?
   
   Since they are fixed once loaded, why don't we cave them in OzoneManager? 
   Let's say in OzoneManager:
   
   ReplicationConfig defaultRepConfig = null;
   
   on startUp, where bucketLayouts validated,
   defaultRepConfig = getDefaultReplicationConfig();
   ReplicationConfigValidator validator =
           conf.getObject(ReplicationConfigValidator.class);
       validator.validate(defaultRepConfig)
   
   Can we do this?



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfigValidator.java:
##########
@@ -50,10 +50,11 @@ public void init() {
 
   public ReplicationConfig validate(ReplicationConfig replicationConfig) {
     if (validationRegexp != null) {
-      if (!validationRegexp.matcher(replicationConfig.toString()).matches()) {
+      if (!validationRegexp.matcher(

Review Comment:
   @kerneltime I think the idea here is to restrict the non-tested combination numbers. So, the list is becoming like a fixed string. However that pattern is dev configurable. 
    @Config(key = "allowed-configs",
         defaultValue = "^((STANDALONE|RATIS)/(ONE|THREE))|(EC/(3-2|6-3|10-4))$",
   
   If we have confidence with other numbers let's say 12-4, then we can simply update above allowed-configs pattern. IIRC, this was the idea.



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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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