Jackie-Jiang commented on code in PR #15725: URL: https://github.com/apache/pinot/pull/15725#discussion_r2093537489
########## pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java: ########## @@ -141,27 +145,40 @@ public TableView getExternalView( public String getSegmentsStatusDetails( @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName, @ApiParam(value = "realtime|offline", required = false) @QueryParam("tableType") String tableTypeStr, + @ApiParam(value = "Show segments being replaced or deleted: true|false", required = false) Review Comment: ```suggestion @ApiParam(value = "Show segments being replaced: true|false", required = false) ``` ########## pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java: ########## @@ -141,27 +145,40 @@ public TableView getExternalView( public String getSegmentsStatusDetails( @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName, @ApiParam(value = "realtime|offline", required = false) @QueryParam("tableType") String tableTypeStr, + @ApiParam(value = "Show segments being replaced or deleted: true|false", required = false) + @QueryParam("showDeletingSegments") Boolean showDeletingSegments, Review Comment: ```suggestion @QueryParam("showReplacedSegments") boolean showReplacedSegments, ``` ########## pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java: ########## @@ -141,27 +145,40 @@ public TableView getExternalView( public String getSegmentsStatusDetails( @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName, @ApiParam(value = "realtime|offline", required = false) @QueryParam("tableType") String tableTypeStr, + @ApiParam(value = "Show segments being replaced or deleted: true|false", required = false) + @QueryParam("showDeletingSegments") Boolean showDeletingSegments, @Context HttpHeaders headers) throws JsonProcessingException { tableName = DatabaseUtils.translateTableName(tableName, headers); TableType tableType = validateTableType(tableTypeStr); TableViews.TableView externalView = getTableState(tableName, TableViews.EXTERNALVIEW, tableType); TableViews.TableView idealStateView = getTableState(tableName, TableViews.IDEALSTATE, tableType); - List<SegmentStatusInfo> segmentStatusInfoListMap = new ArrayList<>(); - segmentStatusInfoListMap = getSegmentStatuses(externalView, idealStateView); + + Map<String, Map<String, String>> externalViewStateMap = getStateMap(externalView); + Map<String, Map<String, String>> idealStateMap = getStateMap(idealStateView); + Set<String> segments = idealStateMap.keySet(); + + SegmentLineage segmentLineage = SegmentLineageAccessHelper + .getSegmentLineage(_pinotHelixResourceManager.getPropertyStore(), tableName); + SegmentLineageUtils + .filterSegmentsBasedOnLineageInPlace(segments, segmentLineage); + Map<String, Map<String, String>> filteredIdealStateMap = idealStateMap.entrySet().stream() + .filter(entry -> segments.contains(entry.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + + List<SegmentStatusInfo> segmentStatusInfoListMap = getSegmentStatuses(externalViewStateMap, filteredIdealStateMap); + return JsonUtils.objectToPrettyString(segmentStatusInfoListMap); } - public List<SegmentStatusInfo> getSegmentStatuses(TableViews.TableView externalView, - TableViews.TableView idealStateView) { - Map<String, Map<String, String>> idealStateMap = getStateMap(idealStateView); - Map<String, Map<String, String>> externalViewMap = getStateMap(externalView); + public List<SegmentStatusInfo> getSegmentStatuses(Map<String, Map<String, String>> externalViewMap, + Map<String, Map<String, String>> idealStateMap) { List<SegmentStatusInfo> segmentStatusInfoList = new ArrayList<>(); - for (Map.Entry<String, Map<String, String>> entry : externalViewMap.entrySet()) { + for (Map.Entry<String, Map<String, String>> entry : idealStateMap.entrySet()) { String segment = entry.getKey(); - Map<String, String> externalViewEntryValue = entry.getValue(); - Map<String, String> idealViewEntryValue = idealStateMap.get(segment); + Map<String, String> externalViewEntryValue = externalViewMap.get(segment); Review Comment: We need to check if this one is `null` ########## pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java: ########## @@ -241,9 +248,40 @@ static public class SegmentSizeDetails { public Map<String, SegmentSizeInfo> _serverInfo = new HashMap<>(); } + private Map<String, List<String>> getServerToSegmentsMap(final String tableNameWithType, Review Comment: (minor) We don't usually put `final` in the arguments ########## pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java: ########## @@ -141,27 +145,40 @@ public TableView getExternalView( public String getSegmentsStatusDetails( @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName, @ApiParam(value = "realtime|offline", required = false) @QueryParam("tableType") String tableTypeStr, + @ApiParam(value = "Show segments being replaced or deleted: true|false", required = false) + @QueryParam("showDeletingSegments") Boolean showDeletingSegments, @Context HttpHeaders headers) throws JsonProcessingException { tableName = DatabaseUtils.translateTableName(tableName, headers); TableType tableType = validateTableType(tableTypeStr); TableViews.TableView externalView = getTableState(tableName, TableViews.EXTERNALVIEW, tableType); TableViews.TableView idealStateView = getTableState(tableName, TableViews.IDEALSTATE, tableType); - List<SegmentStatusInfo> segmentStatusInfoListMap = new ArrayList<>(); - segmentStatusInfoListMap = getSegmentStatuses(externalView, idealStateView); + + Map<String, Map<String, String>> externalViewStateMap = getStateMap(externalView); + Map<String, Map<String, String>> idealStateMap = getStateMap(idealStateView); + Set<String> segments = idealStateMap.keySet(); + + SegmentLineage segmentLineage = SegmentLineageAccessHelper + .getSegmentLineage(_pinotHelixResourceManager.getPropertyStore(), tableName); + SegmentLineageUtils + .filterSegmentsBasedOnLineageInPlace(segments, segmentLineage); + Map<String, Map<String, String>> filteredIdealStateMap = idealStateMap.entrySet().stream() Review Comment: I don't think you need to do this extra step because `segments` is the key set of IS, and IS should already be updated ########## pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java: ########## @@ -241,9 +248,40 @@ static public class SegmentSizeDetails { public Map<String, SegmentSizeInfo> _serverInfo = new HashMap<>(); } + private Map<String, List<String>> getServerToSegmentsMap(final String tableNameWithType, + final Boolean showReplacedSegments) { Review Comment: ```suggestion boolean showReplacedSegments) { ``` ########## pinot-controller/src/main/java/org/apache/pinot/controller/util/TableSizeReader.java: ########## @@ -241,9 +248,40 @@ static public class SegmentSizeDetails { public Map<String, SegmentSizeInfo> _serverInfo = new HashMap<>(); } + private Map<String, List<String>> getServerToSegmentsMap(final String tableNameWithType, + final Boolean showReplacedSegments) { + Map<String, List<String>> serverToSegmentsMap = _helixResourceManager.getServerToSegmentsMap(tableNameWithType); + + Set<String> segments = new HashSet<>(); + for (List<String> segmentList : serverToSegmentsMap.values()) { + segments.addAll(segmentList); + } + + // Filter out replaced segments + if (!showReplacedSegments) { + SegmentLineage segmentLineage = SegmentLineageAccessHelper + .getSegmentLineage(_helixResourceManager.getPropertyStore(), tableNameWithType); + SegmentLineageUtils.filterSegmentsBasedOnLineageInPlace(segments, segmentLineage); + } + + for (Iterator<Map.Entry<String, List<String>>> it = serverToSegmentsMap.entrySet().iterator(); it.hasNext(); ) { + Map.Entry<String, List<String>> entry = it.next(); + List<String> filtered = entry.getValue().stream() + .filter(segments::contains) + .collect(Collectors.toList()); + if (filtered.isEmpty()) { + it.remove(); + } else { + entry.setValue(filtered); + } + } + + return serverToSegmentsMap; + } + public TableSubTypeSizeDetails getTableSubtypeSize(String tableNameWithType, int timeoutMs) throws InvalidConfigException { - Map<String, List<String>> serverToSegmentsMap = _helixResourceManager.getServerToSegmentsMap(tableNameWithType); + Map<String, List<String>> serverToSegmentsMap = getServerToSegmentsMap(tableNameWithType, false); Review Comment: Can we modify the one inside `PinotHelixResourceManager` instead of introducing a new one here? -- 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