slfan1989 commented on PR #10294:
URL: https://github.com/apache/ozone/pull/10294#issuecomment-4475462802

   > Thanks @slfan1989 for the fix.
   > 
   > Please do changes for **calculateVolumeDataDensity — zero-capacity 
volumes**
   > 
   > Filtering to `getUsage().getCapacity() > 0` before calling `getIdealUsage 
/ getUtilization() `is the right fix: V`olumeFixedUsage` leaves `utilization == 
null` when capacity is 0, so the previous loop could only “handle” that path by 
throwing and falling through to catch, which turned a common edge case into 
-1.0 and hid the real issue.
   > 
   > ```
   >   public static double calculateVolumeDataDensity(List<VolumeFixedUsage> 
volumeSet) {
   >     if (volumeSet == null) {
   >       LOG.warn("VolumeSet is null, returning 0.0 for VolumeDataDensity");
   >       return 0.0;
   >     }
   > 
   >     try {
   >       final List<VolumeFixedUsage> usableVolumes = volumeSet.stream()
   >           .filter(v -> v.getUsage().getCapacity() > 0)
   >           .collect(Collectors.toList());
   > 
   >       if (usableVolumes.size() <= 1) {
   >         return 0.0;
   >       }
   > 
   >       // Calculate ideal usage using the same immutable volume snapshot
   >       final double idealUsage = getIdealUsage(usableVolumes);
   >       double volumeDensitySum = 0.0;
   > 
   >       // Calculate density for each volume using the same snapshot
   >       for (VolumeFixedUsage volumeUsage : usableVolumes) {
   >         final double currentUsage = volumeUsage.getUtilization();
   > 
   >         // Calculate density as absolute difference from ideal usage
   >         double volumeDensity = Math.abs(currentUsage - idealUsage);
   >         volumeDensitySum += volumeDensity;
   >       }
   >       return volumeDensitySum;
   >     } catch (Exception e) {
   >       LOG.error("Error calculating VolumeDataDensity", e);
   >       return -1.0;
   >     }
   >   }
   > ```
   > 
   > With this change everywhere for diskbalancer if a volume is of 0 capacity 
that will be excluded.
   
   @Gargi-jais11 Thanks for the suggestion! Updated the test to compare the 
density with and without the zero-capacity volume, which makes the expected 
behavior clearer and avoids the hard-coded density 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