somandal commented on code in PR #16341:
URL: https://github.com/apache/pinot/pull/16341#discussion_r2211378086
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -387,6 +391,24 @@ private RebalanceResult doRebalance(TableConfig
tableConfig, RebalanceConfig reb
tierToInstancePartitionsMap, targetAssignment, preChecksResult,
summaryResult);
}
+ if (downtime &&
!StringUtils.isEmpty(tableConfig.getValidationConfig().getPeerSegmentDownloadScheme()))
{
+ if (!forceDowntime) {
+ // Don't allow downtime rebalance if peer-download is enabled as it
can result in data loss
+ // The best way to rebalance peer-download enabled tables is to:
+ // - Ensure that all segments have their deep-store copy available
+ // - Pause ingestion to prevent the creation of new segments during
rebalance
+ // - set forceDowntime=true and re-try running the rebalance
+ String errorMsg = "Peer-download enabled tables cannot undergo
downtime rebalance due to the potential for "
+ + "data loss, validate all segments exist in deep store and pause
ingestion prior to setting "
+ + "forceDowntime=true to override the downtime flag";
+ tableRebalanceLogger.error(errorMsg);
+ return new RebalanceResult(rebalanceJobId,
RebalanceResult.Status.FAILED, errorMsg, instancePartitionsMap,
Review Comment:
good catch, done
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -411,6 +433,30 @@ private RebalanceResult doRebalance(TableConfig
tableConfig, RebalanceConfig reb
tierToInstancePartitionsMap, rebalanceConfig);
}
}
+
+ // If peer-download is enabled, verify that for all segments with
changes in assignment, it is safe to rebalance
+ // Create the DataLossRiskAssessor which is used to check for data loss
scenarios if peer-download is enabled
+ // for a table
+ if
(!StringUtils.isEmpty(tableConfig.getValidationConfig().getPeerSegmentDownloadScheme()))
{
Review Comment:
same as previous response - this util checks for both null and empty
--
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]
