yashmayya commented on code in PR #19123:
URL: https://github.com/apache/pinot/pull/19123#discussion_r3678862945
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalancePreChecker.java:
##########
@@ -87,6 +91,15 @@ public RebalanceConfig getRebalanceConfig() {
public RebalanceSummaryResult getRebalanceSummaryResult() {
return _rebalanceSummaryResult;
}
+
+ /**
+ * Returns the tier name to segments map computed while updating the
target tiers of this rebalance, or
Review Comment:
Two things I'd add here:
- The map can also be **empty**, not just null — `updateTargetTier` returns
an empty map when the table has tier configs but no segment currently matches a
tier. Callers need to handle both.
- These are the **target** tiers just persisted to ZK, not where the
segments live today. Worth saying so, because for a segment being promoted
local -> remote the two differ and a consumer might assume otherwise.
Also maybe mention that `updateTargetTier` defaults to false, so a plain
`dryRun=true&preChecks=true` request gets null here.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalancePreChecker.java:
##########
@@ -41,11 +42,13 @@ class PreCheckContext {
private final TableSizeReader.TableSubTypeSizeDetails
_tableSubTypeSizeDetails;
private final RebalanceConfig _rebalanceConfig;
private final RebalanceSummaryResult _rebalanceSummaryResult;
+ private final Map<String, Set<String>> _providedTierToSegmentsMap;
Review Comment:
Nothing in OSS exercises this field — `TableRebalancer` is the only thing
that constructs a `PreCheckContext`, and no test does. Since the whole point of
the field is to be read by a downstream pre-checker, a small test that builds a
context with a tier map and asserts it survives to the pre-checker would keep
the plumbing from silently regressing.
--
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]
