somandal commented on code in PR #15817:
URL: https://github.com/apache/pinot/pull/15817#discussion_r2093804392


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java:
##########
@@ -448,10 +472,10 @@ public static int getOfflineSegmentPartitionId(String 
segmentName, String offlin
         
ZKMetadataProvider.getSegmentZKMetadata(helixManager.getHelixPropertyStore(), 
offlineTableName, segmentName);
     Preconditions.checkState(segmentZKMetadata != null,
         "Failed to find segment ZK metadata for segment: %s of table: %s", 
segmentName, offlineTableName);
-    return getPartitionId(segmentZKMetadata, offlineTableName, 
partitionColumn);
+    return getOfflinePartitionId(segmentZKMetadata, offlineTableName, 
partitionColumn);
   }
 
-  private static int getPartitionId(SegmentZKMetadata segmentZKMetadata, 
String offlineTableName,
+  private static int getOfflinePartitionId(SegmentZKMetadata 
segmentZKMetadata, String offlineTableName,
       @Nullable String partitionColumn) {

Review Comment:
   yes, that's one of the issues in why the code changes for this is more 
involved:
   - in most OFFLINE code paths, they check for null partitionColumn or 
numPartitions = 1 outside before calling the getPartitionId util. This is baked 
in deeper than just the code paths that fetch the partitionId. E.g. even 
reassignInstances in replica group does different handling for null 
partitionColumn or numPartitions = 1 and different handling for the else 
scenario
   - for REALTIME, non COMPLETED (i.e. CONSUMING or when relocation is 
disabled), the RealtimeSegmentAssignment doesn't use the numPartitions = 1 
means allocate partitionId = 0 code at all. They always try to calculate a 
partitionId. So we can't generalize this case to use partitionId = 0 when 
numPartitions = 1



-- 
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