klsince commented on code in PR #15368:
URL: https://github.com/apache/pinot/pull/15368#discussion_r2025258481
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalanceSummaryResult.java:
##########
@@ -306,18 +297,120 @@ public Map<String, ServerSegmentChangeInfo>
getServerSegmentChangeInfo() {
}
}
+ public static class ConsumingSegmentToBeMovedSummary {
+ private final int _numConsumingSegmentsToBeMoved;
+ private final int _numServerGettingConsumingSegmentsAdded;
+ private final Map<String, Integer>
_consumingSegmentsToBeMovedWithMostOffsetsToCatchUp;
+ private final Map<String, Integer>
_oldestConsumingSegmentsToBeMovedInMinutes;
+ private final Map<String, ConsumingSegmentSummaryPerServer>
_serverConsumingSegmentSummary;
+
+ /**
+ * Constructor for ConsumingSegmentToBeMovedSummary
+ * @param numConsumingSegmentsToBeMoved total number of consuming segments
to be moved as part of this rebalance
+ * @param numServerGettingConsumingSegmentsAdded maximum bytes of
consuming segments to be moved to catch up
+ * @param consumingSegmentsToBeMovedWithMostOffsetsToCatchUp top consuming
segments to be moved to catch up.
Review Comment:
iiuc about this stmt "...difference between the latest offset of the stream
and the segment's start offset of the stream..", then the oldest consuming
segments would be the ones lagging most (as they would have the smallest start
offsets), if so this Map and the one below would be duplicate?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -806,6 +864,157 @@ private List<String> getServerTag(String serverName) {
return instanceConfig.getTags();
}
+ private RebalanceSummaryResult.ConsumingSegmentToBeMovedSummary
getConsumingSegmentSummary(String tableNameWithType,
Review Comment:
how about creating a class like ConsumingSegmentInfoReader (like
tableSizeReader) and pass the reader object to the TableRebalancer, instead of
passing the deps (executor, connMgr, and pinotResMgr) the reader needs.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -243,6 +244,8 @@ private enum LineageUpdateType {
private final LineageManager _lineageManager;
private final RebalancePreChecker _rebalancePreChecker;
private TableSizeReader _tableSizeReader;
+ private final ExecutorService _executorService;
+ private HttpClientConnectionManager _connectionManager;
Review Comment:
afaik, most classes reuse the executor/connectionmgr created by
ControllerStarter.
Restlet classes use dependency injection to get access to those objects,
thus looks cleaner. For PinotHelixResourceManager, looks like the convention is
to create register...() methods to get access to those objects, so it looks
fine to me to use register...() methods 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
