somandal commented on code in PR #15284:
URL: https://github.com/apache/pinot/pull/15284#discussion_r2003996827
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalanceSummaryResult.java:
##########
@@ -39,6 +39,8 @@ public class RebalanceSummaryResult {
private final ServerInfo _serverInfo;
@JsonInclude(JsonInclude.Include.NON_NULL)
private final SegmentInfo _segmentInfo;
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ private final List<TenantInfo> _tenantsInfo;
Review Comment:
nit: can we call this `tenantInfo` instead?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -670,6 +686,20 @@ private RebalanceSummaryResult
calculateDryRunSummary(Map<String, Map<String, St
serverSegmentChangeInfoMap.put(server, new
RebalanceSummaryResult.ServerSegmentChangeInfo(serverStatus,
totalNewSegments, totalExistingSegments, segmentsAdded,
segmentsDeleted, segmentsUnchanged,
instanceToTagsMap.getOrDefault(server, null)));
+ List<String> serverTags = getServerTag(server);
+ // Since this is a server in the target assignment, it should contain at
least one tag of the tenant or tier
+ // server tag. Note that if the server is tagged with multiple tenant or
tier tags that are used in the table
+ // config, we will count it multiple times, i.e. the total segment count
would not add up to the actual total.
+ assert serverTags != null;
Review Comment:
can we avoid assert and set some special error status instead? while it is
good to try and get summary updated, we probably don't want to fail rebalance
or dryRun at this stage. maybe we can track `numServersWithoutTags`?
also, it is possible `serverTags` is not null, but has size 0 or doesn't
have one of the expected tags - what's your intention with that scenario?
I understand this shouldn't happen, otherwise how'd this server get
selected, but I guess there could be race conditions where someone issued
rebalance and untagged servers etc.
--
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
