slfan1989 commented on code in PR #11503:
URL: https://github.com/apache/ozone/pull/11503#discussion_r4267746975


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerConfiguration.java:
##########
@@ -169,6 +181,28 @@ public DiskBalancerConfiguration(Double threshold,
   public DiskBalancerConfiguration() {
   }
 
+  @PostConstruct
+  public void validate() {
+    if (threshold <= 0d || threshold >= 100d) {

Review Comment:
   `NaN` bypasses this validation because both comparisons evaluate to `false`. 
   
   Since the configuration parser uses `Double.parseDouble()`, an XML value of 
`NaN` reaches this method and is subsequently persisted to `diskBalancer.info`.
   
   Once balancing runs, comparisons against the resulting `NaN` threshold are 
always false. For example, `newUtilization >= upperThreshold` cannot reject a 
move, which may cause containers to be selected without a valid utilization 
limit.
   
   Could we reject non-finite values here and in `setThreshold()`, for example:
   
   ```
   if (!Double.isFinite(threshold) || threshold <= 0d || threshold >= 100d)
   ```
   
   Please also add a test using a raw `NaN` configuration value.



-- 
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: [email protected]

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