somandal commented on code in PR #15317:
URL: https://github.com/apache/pinot/pull/15317#discussion_r2013234329
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java:
##########
@@ -826,24 +826,48 @@ public void testRebalancePreChecks()
checkRebalancePreCheckStatus(rebalanceResult, RebalanceResult.Status.NO_OP,
"Instance assignment not allowed, no need for minimizeDataMovement",
RebalancePreCheckerResult.PreCheckStatus.PASS, "No need to reload",
+ RebalancePreCheckerResult.PreCheckStatus.PASS, "No rebalance
parameters need to double check",
RebalancePreCheckerResult.PreCheckStatus.PASS);
// Enable minimizeDataMovement
tableConfig.setInstanceAssignmentConfigMap(createInstanceAssignmentConfigMap(true));
rebalanceResult = _tableRebalancer.rebalance(tableConfig, rebalanceConfig,
null);
checkRebalancePreCheckStatus(rebalanceResult, RebalanceResult.Status.NO_OP,
"minimizeDataMovement is enabled",
RebalancePreCheckerResult.PreCheckStatus.PASS,
- "No need to reload", RebalancePreCheckerResult.PreCheckStatus.PASS);
+ "No need to reload", RebalancePreCheckerResult.PreCheckStatus.PASS,
+ "No rebalance parameters need to double check",
+ RebalancePreCheckerResult.PreCheckStatus.PASS);
+
+ // Override minimizeDataMovement
+
rebalanceConfig.setMinimizeDataMovement(RebalanceConfig.MinimizeDataMovementOptions.DISABLE);
+ rebalanceResult = _tableRebalancer.rebalance(tableConfig, rebalanceConfig,
null);
+ checkRebalancePreCheckStatus(rebalanceResult, RebalanceResult.Status.NO_OP,
+ "minimizeDataMovement is enabled in table config but it's overridden
with disabled",
+ RebalancePreCheckerResult.PreCheckStatus.WARN, "No need to reload",
+ RebalancePreCheckerResult.PreCheckStatus.PASS, "No rebalance
parameters need to double check",
+ RebalancePreCheckerResult.PreCheckStatus.PASS);
Review Comment:
recommend doing one of the following:
- add just 1 test that exercises this to return a WARN, or
- Remove these as parameters and just have the assert for both in
`checkRebalancePreCheckStatus` since it looks like these two values are fixed
in this test
--
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]
