Jackie-Jiang commented on code in PR #15725:
URL: https://github.com/apache/pinot/pull/15725#discussion_r2080298572
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java:
##########
@@ -144,11 +148,28 @@ public String getSegmentsStatusDetails(
@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);
+ final TableType tableType = validateTableType(tableTypeStr);
+ final TableViews.TableView externalView = getTableState(tableName,
TableViews.EXTERNALVIEW, tableType);
+ final TableViews.TableView idealStateView = getTableState(tableName,
TableViews.IDEALSTATE, tableType);
+
List<SegmentStatusInfo> segmentStatusInfoListMap = new ArrayList<>();
Review Comment:
Not introduced in this PR, but this initialization is redundant
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java:
##########
@@ -144,11 +148,28 @@ public String getSegmentsStatusDetails(
@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);
+ final TableType tableType = validateTableType(tableTypeStr);
Review Comment:
(minor, convention) We don't usually use `final` for local variable
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java:
##########
@@ -144,11 +148,28 @@ public String getSegmentsStatusDetails(
@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);
+ final TableType tableType = validateTableType(tableTypeStr);
+ final TableViews.TableView externalView = getTableState(tableName,
TableViews.EXTERNALVIEW, tableType);
+ final TableViews.TableView idealStateView = getTableState(tableName,
TableViews.IDEALSTATE, tableType);
+
List<SegmentStatusInfo> segmentStatusInfoListMap = new ArrayList<>();
segmentStatusInfoListMap = getSegmentStatuses(externalView,
idealStateView);
Review Comment:
Not introduced in this PR, but we should loop over segments in IS instead of
EV. We can modify the handling to pass in a set of segments to check, and first
filter segments based on lineage before checking their status to reduce overhead
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java:
##########
@@ -144,11 +148,28 @@ public String getSegmentsStatusDetails(
@Context HttpHeaders headers)
Review Comment:
Consider adding a param to show segments being replaced for debugging purpose
--
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
