[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11585: Enhance SegmentPartitionMetadataManager to handle new segment

["Jackie-Jiang (via GitHub)" <gi...@apache.org>]

Jackie-Jiang commented on code in PR #11585:
URL: https://github.com/apache/pinot/pull/11585#discussion_r1325041098


##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpartition/SegmentPartitionMetadataManager.java:
##########
@@ -151,8 +176,48 @@ private void computeTablePartitionInfo() {
         LOGGER.warn("Found {} segments: {} with invalid partition from table: {}", numSegmentsWithInvalidPartition,
             segmentsWithInvalidPartition, _tableNameWithType);
       } else {
-        LOGGER.warn("Found {} segments: {} with invalid partition from table: {}", numSegmentsWithInvalidPartition,
-            segmentsWithInvalidPartition, _tableNameWithType);
+        LOGGER.warn("Found {} segments: {}... with invalid partition from table: {}", numSegmentsWithInvalidPartition,
+            segmentsWithInvalidPartition.subList(0, 10), _tableNameWithType);
+      }
+    }
+    // Process new segments
+    if (!newSegmentInfoEntries.isEmpty()) {
+      List<String> excludedNewSegments = new ArrayList<>();
+      for (Map.Entry<String, SegmentInfo> entry : newSegmentInfoEntries) {
+        String segment = entry.getKey();
+        SegmentInfo segmentInfo = entry.getValue();
+        int partitionId = segmentInfo._partitionId;
+        List<String> onlineServers = segmentInfo._onlineServers;
+        PartitionInfo partitionInfo = partitionInfoMap[partitionId];
+        if (partitionInfo == null) {
+          // If the new segment is the first segment of a partition, treat it as regular segment
+          Set<String> fullyReplicatedServers = new HashSet<>(onlineServers);
+          List<String> segments = new ArrayList<>();
+          segments.add(segment);
+          partitionInfo = new PartitionInfo(fullyReplicatedServers, segments);
+          partitionInfoMap[partitionId] = partitionInfo;
+        } else {
+          // If the new segment is not the first segment of a partition, add it only if it won't reduce the fully
+          // replicated servers. It is common that a new segment (newly pushed, or a new consuming segment) doesn't have
+          // all the replicas available yet, and we want to exclude it from the partition info until all the replicas
+          // are available.
+          //noinspection SlowListContainsAll

Review Comment:
   Segment is count as new for up to 5 minutes after pushed/created, so committing segment won't be counted as new segment. New consuming segment is created after the previous consuming segment is committed.



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org