somandal commented on code in PR #15822:
URL: https://github.com/apache/pinot/pull/15822#discussion_r2096434650
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -1815,13 +1818,27 @@ public int fetch(String segmentName) {
_helixManager, _partitionColumn);
}
} else {
- // This how partitionId is calculated for CONSUMING segments in
RealtimeSegmentAssignment
- // TODO: Add handling for COMPLETED segments if in the future this
is allowed for StrictReplicaGroup and
- // the partitionId calculation differs from the CONSUMING
segments. For StrictRealtimeSegmentAssignment
- // the partitionId is mandated today. If this mandate is
maintained then there may be no need to add
- // special handling for COMPLETED segments after all
- partitionId =
SegmentAssignmentUtils.getRealtimeSegmentPartitionId(segmentName,
_tableNameWithType,
- _helixManager, _partitionColumn);
+ if (isConsuming ||
!_instancePartitionsMap.containsKey(InstancePartitionsType.COMPLETED)) {
+ // This how partitionId is calculated for CONSUMING segments and
ONLINE segments without COMPLETED
Review Comment:
done
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -1815,13 +1818,27 @@ public int fetch(String segmentName) {
_helixManager, _partitionColumn);
}
} else {
- // This how partitionId is calculated for CONSUMING segments in
RealtimeSegmentAssignment
- // TODO: Add handling for COMPLETED segments if in the future this
is allowed for StrictReplicaGroup and
- // the partitionId calculation differs from the CONSUMING
segments. For StrictRealtimeSegmentAssignment
- // the partitionId is mandated today. If this mandate is
maintained then there may be no need to add
- // special handling for COMPLETED segments after all
- partitionId =
SegmentAssignmentUtils.getRealtimeSegmentPartitionId(segmentName,
_tableNameWithType,
- _helixManager, _partitionColumn);
+ if (isConsuming ||
!_instancePartitionsMap.containsKey(InstancePartitionsType.COMPLETED)) {
+ // This how partitionId is calculated for CONSUMING segments and
ONLINE segments without COMPLETED
+ // instance partitions in RealtimeSegmentAssignment
+ partitionId =
SegmentAssignmentUtils.getRealtimeSegmentPartitionId(segmentName,
_tableNameWithType,
+ _helixManager, _partitionColumn);
+ } else {
+ // This is how partitionId is calculated for ONLINE segments when
COMPLETED instance partitions exist
+ // in RealtimeSegmentAssignment
+ InstancePartitions instancePartitions =
_instancePartitionsMap.get(InstancePartitionsType.COMPLETED);
+ assert instancePartitions != null;
+ if (_partitionColumn == null ||
instancePartitions.getNumPartitions() == 1) {
+ // Fallback to partitionId 0, in this case batching will not be
possible so we will fall back to a full
+ // rebalance without batching
+ partitionId = 0;
+ } else {
+ // This how partitionId is calculated for REALTIME tables if a
partition column exists and if the
Review Comment:
done
--
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
