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

Reply via email to