J-HowHuang commented on code in PR #17027:
URL: https://github.com/apache/pinot/pull/17027#discussion_r2437475449
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/tenant/TenantRebalancer.java:
##########
@@ -355,11 +356,19 @@ ConcurrentLinkedDeque<TenantTableRebalanceJobContext>
createTableQueue(TenantReb
Queue<TenantTableRebalanceJobContext> lastQueue = new LinkedList<>();
Set<String> dimTables = getDimensionalTables(config.getTenantName());
dryRunResults.forEach((table, dryRunResult) -> {
- TenantTableRebalanceJobContext jobContext =
- new TenantTableRebalanceJobContext(table, dryRunResult.getJobId(),
dryRunResult.getRebalanceSummaryResult()
- .getSegmentInfo()
- .getReplicationFactor()
- .getExpectedValueAfterRebalance() == 1);
+ TenantTableRebalanceJobContext jobContext;
+ if (dryRunResult.getStatus() == RebalanceResult.Status.FAILED) {
+ jobContext = new TenantTableRebalanceJobContext(table,
dryRunResult.getJobId(), false);
+ LOGGER.warn("User proceeded with rebalancing table: {} despite its
failed dry-run", table);
Review Comment:
Fair point, reworded.
One only failure (except exceptions) I can see and I did see is rebalancing
a disabled table w/o `downtime=true`. We do have mechanism to rebalance RF=1
tables with `minAvailableReplicas=0` but not rebalancing disabled tables with
`downtime=true`. For now I think it's reasonable to ignore the rebalance
failure on these disabled table and proceed a tenant rebalance regardless.
--
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]
