klsince commented on code in PR #15175:
URL: https://github.com/apache/pinot/pull/15175#discussion_r1994254289


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -138,4 +157,80 @@ private boolean checkIsMinimizeDataMovement(String 
rebalanceJobId, String tableN
       return false;
     }
   }
+
+  private String checkDiskUtilization(String tableNameWithType, Map<String, 
Map<String, String>> currentAssignment,

Review Comment:
   I think this method may be refactored to just get disk utils, and the method 
caller can decide what to do with the utils, e.g. checking utils against the 
threshold



##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -42,19 +47,28 @@ public class DefaultRebalancePreChecker implements 
RebalancePreChecker {
 
   public static final String NEEDS_RELOAD_STATUS = "needsReloadStatus";
   public static final String IS_MINIMIZE_DATA_MOVEMENT = 
"isMinimizeDataMovement";
+  public static final String DISK_UTILIZATION = "diskUtilization";
+
+  private static double _diskUtilizationThreshold;
 
   protected PinotHelixResourceManager _pinotHelixResourceManager;
   protected ExecutorService _executorService;
 
   @Override
-  public void init(PinotHelixResourceManager pinotHelixResourceManager, 
@Nullable ExecutorService executorService) {
+  public void init(PinotHelixResourceManager pinotHelixResourceManager, 
@Nullable ExecutorService executorService,
+      double diskUtilizationThreshold) {
     _pinotHelixResourceManager = pinotHelixResourceManager;
     _executorService = executorService;
+    _diskUtilizationThreshold = diskUtilizationThreshold;
   }
 
   @Override
-  public Map<String, String> check(String rebalanceJobId, String 
tableNameWithType, TableConfig tableConfig) {
-    LOGGER.info("Start pre-checks for table: {} with rebalanceJobId: {}", 
tableNameWithType, rebalanceJobId);
+  public Map<String, String> check(TableFacts tableFacts) {

Review Comment:
   It might be more flexible to pass thresholds to this check method, instead 
of passing it to the init method, so that caller can use different thresholds 
when calling check().



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to