somandal commented on code in PR #15527:
URL: https://github.com/apache/pinot/pull/15527#discussion_r2040352447
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -227,9 +228,31 @@ private RebalancePreCheckerResult
checkIsMinimizeDataMovement(String rebalanceJo
return RebalancePreCheckerResult.warn(
"minimizeDataMovement may not be enabled for consuming or
completed, but instance assigment is allowed "
Review Comment:
nit: not a mistake in your PR, but can you fix spelling `assigment` to
`assignment`
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -227,9 +228,31 @@ private RebalancePreCheckerResult
checkIsMinimizeDataMovement(String rebalanceJo
return RebalancePreCheckerResult.warn(
"minimizeDataMovement may not be enabled for consuming or
completed, but instance assigment is allowed "
+ "for both");
+ } else if (instanceAssignmentConfigConsuming != null) {
+ if (rebalanceConfig.getMinimizeDataMovement() ==
RebalanceConfig.MinimizeDataMovementOptions.ENABLE) {
+ return RebalancePreCheckerResult.pass("minimizeDataMovement is
enabled");
+ }
+ if (instanceAssignmentConfigConsuming.isMinimizeDataMovement()) {
+ return rebalanceConfig.getMinimizeDataMovement() ==
RebalanceConfig.MinimizeDataMovementOptions.DISABLE
+ ? RebalancePreCheckerResult.warn("minimizeDataMovement is
enabled in table config but it's overridden "
+ + "with disabled")
+ : RebalancePreCheckerResult.pass("minimizeDataMovement is
enabled");
+ }
+ return RebalancePreCheckerResult.warn(
+ "minimizeDataMovement is not enabled for consuming segments, but
instance assignment is allowed");
+ } else {
+ if (rebalanceConfig.getMinimizeDataMovement() ==
RebalanceConfig.MinimizeDataMovementOptions.ENABLE) {
+ return RebalancePreCheckerResult.pass("minimizeDataMovement is
enabled");
+ }
+ if (instanceAssignmentConfigCompleted.isMinimizeDataMovement()) {
+ return rebalanceConfig.getMinimizeDataMovement() ==
RebalanceConfig.MinimizeDataMovementOptions.DISABLE
+ ? RebalancePreCheckerResult.warn("minimizeDataMovement is
enabled in table config but it's overridden "
Review Comment:
same, let's call out this is for COMPLETED
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -227,9 +228,31 @@ private RebalancePreCheckerResult
checkIsMinimizeDataMovement(String rebalanceJo
return RebalancePreCheckerResult.warn(
"minimizeDataMovement may not be enabled for consuming or
completed, but instance assigment is allowed "
+ "for both");
+ } else if (instanceAssignmentConfigConsuming != null) {
+ if (rebalanceConfig.getMinimizeDataMovement() ==
RebalanceConfig.MinimizeDataMovementOptions.ENABLE) {
+ return RebalancePreCheckerResult.pass("minimizeDataMovement is
enabled");
+ }
+ if (instanceAssignmentConfigConsuming.isMinimizeDataMovement()) {
+ return rebalanceConfig.getMinimizeDataMovement() ==
RebalanceConfig.MinimizeDataMovementOptions.DISABLE
+ ? RebalancePreCheckerResult.warn("minimizeDataMovement is
enabled in table config but it's overridden "
Review Comment:
shall we call out this is for CONSUMING?
--
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
