Jackie-Jiang commented on code in PR #15817:
URL: https://github.com/apache/pinot/pull/15817#discussion_r2122191645
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/SegmentUtils.java:
##########
@@ -33,38 +34,42 @@ public class SegmentUtils {
private SegmentUtils() {
}
- // Returns the partition id of a realtime segment based segment name and
segment metadata info retrieved via Helix.
- // Important: The method is costly because it may read data from zookeeper.
Do not use it in any query execution
- // path.
+ /// Returns the partition id of a segment based on segment name or ZK
metadata.
+ /// Can return `null` if the partition id cannot be determined.
+ /// Important: The method is costly because it may read data from zookeeper.
Do not use it in query execution path.
@Nullable
- public static Integer getRealtimeSegmentPartitionId(String segmentName,
String realtimeTableName,
- HelixManager helixManager, @Nullable String partitionColumn) {
- Integer partitionId = getPartitionIdFromRealtimeSegmentName(segmentName);
+ public static Integer getSegmentPartitionId(String segmentName, String
tableNameWithType, HelixManager helixManager,
Review Comment:
This is intentional for future improvement to integrate partition id into
segment name and reduce ZK access. In the long term we want to model `OFFLINE`
table as `REALTIME` table without consuming segments, and keep the common
handling logic as much as possible. People might push LLC segment to `OFFLINE`
table, and we can follow the same convention.
Good catch that `LLCSegmentName.of()` doesn't tolerate illegal
partition/sequence id. Added a commit to make it same as
`UploadedRealtimeSegmentName.of()`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
