[GitHub] [druid] suneet-s commented on a change in pull request #10724: Coordinator Dynamic Config changes to ease upgrading with new config value

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

suneet-s commented on a change in pull request #10724:
URL: https://github.com/apache/druid/pull/10724#discussion_r551456475



##########
File path: server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java
##########
@@ -126,6 +129,15 @@ public CoordinatorDynamicConfig(
     this.mergeSegmentsLimit = mergeSegmentsLimit;
     this.maxSegmentsToMove = maxSegmentsToMove;
 
+    if (percentOfSegmentsToConsiderPerMove == null) {
+      log.warn("percentOfSegmentsToConsiderPerMove was null! This is likely because your metastore does not "

Review comment:
       I don't think this should be a warn log message. A user should not be expected to set an optional config. If the config is not set, it seems reasonable to me that the default is set to preserve the old behavior (ie all segments are considered per move). 
   
   For users who want to use the new behavior, I suspect they will hear about it in the release notes or the docs, and update the config to get the new functionality.
    
   ```suggestion
         log.debug("percentOfSegmentsToConsiderPerMove was null! This is likely because your metastore does not "
   ```




----------------------------------------------------------------
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org