klsince commented on code in PR #15175: URL: https://github.com/apache/pinot/pull/15175#discussion_r2003988748
########## pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java: ########## @@ -42,19 +47,27 @@ 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_DURING_REBALANCE = "diskUtilizationDuringRebalance"; + public static final String DISK_UTILIZATION_AFTER_REBALANCE = "diskUtilizationAfterRebalance"; + + 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, RebalancePreCheckerResult> check(String rebalanceJobId, String tableNameWithType, - TableConfig tableConfig) { + public Map<String, RebalancePreCheckerResult> check(PreCheckContext preCheckContext) { + String rebalanceJobId = preCheckContext._rebalanceJobId; Review Comment: better use getter methods to access those fields, as this class is not an inner class of this Checker class. ########## 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: I see PreCheckContext was added, so should the threshold be passed via this context to the check() method, instead of passing it via the init()? -- 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