somandal commented on code in PR #15284: URL: https://github.com/apache/pinot/pull/15284#discussion_r2010790936
########## pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java: ########## @@ -699,6 +714,26 @@ 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. + if (serverTags.isEmpty()) { + LOGGER.warn( Review Comment: should we have a top-level field in tenantInfo which tracks the number of such servers? e.g.: TenantsInfo: - numServersWithNoTags - numServersWithNoRelevantTags (i.e. those that have tags but none match the TableConfig server or tier tenant tags) - List of current tenant info (which you already have) with the above, folks won't need to check the logs to find this warning, otherwise it is easy to miss -- 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